I agree with dan (and from your reply below - it looks like you also agree). The synchronization looks right to me.
When reading the raw store page code, remember that most of the synchonization at the page level is by getting a latch on the page for the duration of the page operation - thus you won't see a lot of sync blocks. As dan points out the sync blocks you see are going to be protecting one page from another page while reading from the file shared by both.
Could you mark the jira entry not a bug (or whatever the jira equivalent is) - or let me know if you don't have permission to do so.
/mikem
RPost wrote:
You are correct. Brain fart! Fix - new supply of Beano. ----- Original Message ----- From: "Daniel John Debrunner (JIRA)" <[email protected]> To: <[EMAIL PROTECTED]> Sent: Saturday, February 12, 2005 9:46 PM Subject: [jira] Commented: (DERBY-145) RAFContainer readPage method is not thread safe
[
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
