Thanks. For some reason I forgot that locals are allocated on the stack. I was going to close the 'bug' but I don't have permission to.
----- Original Message ----- From: "Mike Matrigali" <[EMAIL PROTECTED]> To: "Derby Development" <[email protected]> Sent: Monday, February 14, 2005 10:55 AM Subject: Re: [jira] Commented: (DERBY-145) RAFContainer readPage method is not thread safe > 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 > >> > > > > > > > > >
