You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by "Rick Post (JIRA)" <de...@db.apache.org> on 2005/02/13 02:11:11 UTC

[jira] Created: (DERBY-145) RAFContainer readPage method is not thread safe

RAFContainer readPage method is not thread safe
-----------------------------------------------

         Key: DERBY-145
         URL: http://issues.apache.org/jira/browse/DERBY-145
     Project: Derby
        Type: Bug
  Components: Store  
    Versions: 10.0.2.1    
 Environment: N/A
    Reporter: Rick Post
    Priority: Minor


readPage method comment says 'thread safe and has a synchronized block.

But 'pageOffset' computation occurs outside the sync block as does decryption.

This allows the (remote?) possibility of part of the operation(s) being performed using the wrong pageNumber or pageData.

Fix - synchronize the method rather than the block

	/**
		Read a page into the supplied array.

		<BR> MT - thread safe
		@exception IOException exception reading page
		@exception StandardException Standard Cloudscape error policy
	*/
	protected void readPage(long pageNumber, byte[] pageData)
		 throws IOException, StandardException
	{
		if (SanityManager.DEBUG) {
			SanityManager.ASSERT(!getCommittedDropState());
		}

		long pageOffset = pageNumber * pageSize;

		synchronized (this) {

			fileData.seek(pageOffset);

			fileData.readFully(pageData, 0, pageSize);
		}

		if (dataFactory.databaseEncrypted() &&
			pageNumber != FIRST_ALLOC_PAGE_NUMBER)
		{
			decryptPage(pageData, pageSize);
		}
	}


-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


[jira] Closed: (DERBY-145) RAFContainer readPage method is not thread safe

Posted by "Mike Matrigali (JIRA)" <de...@db.apache.org>.
     [ http://issues.apache.org/jira/browse/DERBY-145?page=history ]
     
Mike Matrigali closed DERBY-145:
--------------------------------

     Assign To: Mike Matrigali
    Resolution: Won't Fix

> RAFContainer readPage method is not thread safe
> -----------------------------------------------
>
>          Key: DERBY-145
>          URL: http://issues.apache.org/jira/browse/DERBY-145
>      Project: Derby
>         Type: Bug
>   Components: Store
>     Versions: 10.0.2.1
>  Environment: N/A
>     Reporter: Rick Post
>     Assignee: Mike Matrigali
>     Priority: Minor

>
> readPage method comment says 'thread safe and has a synchronized block.
> But 'pageOffset' computation occurs outside the sync block as does decryption.
> This allows the (remote?) possibility of part of the operation(s) being performed using the wrong pageNumber or pageData.
> Fix - synchronize the method rather than the block
> 	/**
> 		Read a page into the supplied array.
> 		<BR> MT - thread safe
> 		@exception IOException exception reading page
> 		@exception StandardException Standard Cloudscape error policy
> 	*/
> 	protected void readPage(long pageNumber, byte[] pageData)
> 		 throws IOException, StandardException
> 	{
> 		if (SanityManager.DEBUG) {
> 			SanityManager.ASSERT(!getCommittedDropState());
> 		}
> 		long pageOffset = pageNumber * pageSize;
> 		synchronized (this) {
> 			fileData.seek(pageOffset);
> 			fileData.readFully(pageData, 0, pageSize);
> 		}
> 		if (dataFactory.databaseEncrypted() &&
> 			pageNumber != FIRST_ALLOC_PAGE_NUMBER)
> 		{
> 			decryptPage(pageData, pageSize);
> 		}
> 	}

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


[jira] Commented: (DERBY-145) RAFContainer readPage method is not thread safe

Posted by "Rick Post (JIRA)" <de...@db.apache.org>.
     [ http://issues.apache.org/jira/browse/DERBY-145?page=comments#action_59101 ]
     
Rick Post commented on DERBY-145:
---------------------------------

These other methods may have a similar problem:

writeRAFHeader - not synced but called from public run method and other synced methods

clean      - 'waited' variable set prior to 'sync'


> RAFContainer readPage method is not thread safe
> -----------------------------------------------
>
>          Key: DERBY-145
>          URL: http://issues.apache.org/jira/browse/DERBY-145
>      Project: Derby
>         Type: Bug
>   Components: Store
>     Versions: 10.0.2.1
>  Environment: N/A
>     Reporter: Rick Post
>     Priority: Minor

>
> readPage method comment says 'thread safe and has a synchronized block.
> But 'pageOffset' computation occurs outside the sync block as does decryption.
> This allows the (remote?) possibility of part of the operation(s) being performed using the wrong pageNumber or pageData.
> Fix - synchronize the method rather than the block
> 	/**
> 		Read a page into the supplied array.
> 		<BR> MT - thread safe
> 		@exception IOException exception reading page
> 		@exception StandardException Standard Cloudscape error policy
> 	*/
> 	protected void readPage(long pageNumber, byte[] pageData)
> 		 throws IOException, StandardException
> 	{
> 		if (SanityManager.DEBUG) {
> 			SanityManager.ASSERT(!getCommittedDropState());
> 		}
> 		long pageOffset = pageNumber * pageSize;
> 		synchronized (this) {
> 			fileData.seek(pageOffset);
> 			fileData.readFully(pageData, 0, pageSize);
> 		}
> 		if (dataFactory.databaseEncrypted() &&
> 			pageNumber != FIRST_ALLOC_PAGE_NUMBER)
> 		{
> 			decryptPage(pageData, pageSize);
> 		}
> 	}

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


[jira] Commented: (DERBY-145) RAFContainer readPage method is not thread safe

Posted by "Daniel John Debrunner (JIRA)" <de...@db.apache.org>.
     [ http://issues.apache.org/jira/browse/DERBY-145?page=comments#action_59105 ]
     
Daniel John Debrunner commented on DERBY-145:
---------------------------------------------

readPage() is correctly thread safe. The calculation of pageOffset is a local variable and references a local variable pageNumber and a constant pageSize (for the lifetime of the object), thus it does not need to be synchronized. The synchronized block is there to prevent multiple callers of readPage from modifying the file descriptor fileData's position used to read the page from file. I.e if multiple callers were allowed into that small synchronized section then the offset could change between setting it and reading the data, leading to the wrong page being read in. Comments could probably be added to the code.

> RAFContainer readPage method is not thread safe
> -----------------------------------------------
>
>          Key: DERBY-145
>          URL: http://issues.apache.org/jira/browse/DERBY-145
>      Project: Derby
>         Type: Bug
>   Components: Store
>     Versions: 10.0.2.1
>  Environment: N/A
>     Reporter: Rick Post
>     Priority: Minor

>
> readPage method comment says 'thread safe and has a synchronized block.
> But 'pageOffset' computation occurs outside the sync block as does decryption.
> This allows the (remote?) possibility of part of the operation(s) being performed using the wrong pageNumber or pageData.
> Fix - synchronize the method rather than the block
> 	/**
> 		Read a page into the supplied array.
> 		<BR> MT - thread safe
> 		@exception IOException exception reading page
> 		@exception StandardException Standard Cloudscape error policy
> 	*/
> 	protected void readPage(long pageNumber, byte[] pageData)
> 		 throws IOException, StandardException
> 	{
> 		if (SanityManager.DEBUG) {
> 			SanityManager.ASSERT(!getCommittedDropState());
> 		}
> 		long pageOffset = pageNumber * pageSize;
> 		synchronized (this) {
> 			fileData.seek(pageOffset);
> 			fileData.readFully(pageData, 0, pageSize);
> 		}
> 		if (dataFactory.databaseEncrypted() &&
> 			pageNumber != FIRST_ALLOC_PAGE_NUMBER)
> 		{
> 			decryptPage(pageData, pageSize);
> 		}
> 	}

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira