[
https://issues.apache.org/jira/browse/DERBY-700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12504721
]
Daniel John Debrunner commented on DERBY-700:
---------------------------------------------
Note I'm reviewing the patch called; derby-700_06_07_07_diff.txt
Comments on 6/12 seem to indicate a new patch for review but nothing seems to
have been attached that day, I think those comments are about the previous
version.
Some comments on the patch:
- In getIntraJvmDbLock() there is this comment:
+ // synchronizing across the same database, by using interned
+ // version of the database name
but I can't see any synchronization. It seems the synchronization has moved
to wrap the entire call to privGetJBMSLockOnDB().
Since this synchronization is key it would be good to get the comments clear
so that future readers can understand how this works.
Also I think putting the synchronization in the privGetJBMSLockOnDB() method
would be much clearer than hiding it in the run() method.
I see the comment above indicating the synchronization was expanded to be the
full privGetJBMSLockOnDB() method, but there is
no reason given. Can you share your thoughts on why this is required?
- the synchronization for getting the intra jvm lock and releasing it are using
different objects. Don't they need to use the same object?
If not it should be clearly commented how the synchronization is working.
- In getIntraJvmDbLock() and releaseIntraJvmDbLock() the StorageAccessFile
lckFileRaf is never closed, I think it needs to be or have comments indicating
why it is not closed. Maybe it's related to the issue of the new method added
to StorageFile??
- In releaseIntraJvmDbLock() I was a little unclear by the invalid uuid
concept. Is it an invalid uuid such that recreateUUID will throw an exception
in getIntraJvmDbLock() or is it a valid representation of a uuid that can never
be created by Derby?
- Using dataDirectory.intern() to synchronize across class loaders in
releaseIntraJvmDbLock() could probably use more comments, basically describing
what guarantees that all derby systems have the same path for the data
directory for a given database?
Thanks for working on this, it will help stop users corrupting their own
databases.
> Derby does not prevent dual boot of database from different classloaders on
> Linux
> ---------------------------------------------------------------------------------
>
> Key: DERBY-700
> URL: https://issues.apache.org/jira/browse/DERBY-700
> Project: Derby
> Issue Type: Bug
> Components: Store
> Affects Versions: 10.1.2.1
> Environment: ava -version
> java version "1.4.2_08"
> Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.2_08-b03)
> Java HotSpot(TM) Client VM (build 1.4.2_08-b03, mixed mode)
> Reporter: Kathey Marsden
> Assignee: Kathey Marsden
> Priority: Critical
> Fix For: 10.3.0.0
>
> Attachments: DERBY-700.diff, DERBY-700.stat,
> derby-700_06_07_07_diff.txt, derby-700_06_07_07_stat.txt, derby-700_diff.txt,
> derby-700_stat.txt, DERBY-700_v1_use_to_run_DualBootrepro_multithreaded.diff,
> DERBY-700_v1_use_to_run_DualBootrepro_multithreaded.stat,
> derby-700_with_NPE_fix_diff.txt, derby-700_with_NPE_fix_stat.txt, derby.log,
> derby700_singleproperty_v1.diff, derby700_singleproperty_v1.stat,
> DualBootRepro.java, DualBootRepro2.zip, DualBootRepro_mutltithreaded.tar.bz2,
> releaseNote.html
>
>
> Derby does not prevent dual boot from two different classloaders on Linux.
> To reproduce run the program DualBootRepro with no derby jars in your
> classpath. The program assumes derby.jar is in 10.1.2.1/derby.jar, you can
> change the location by changing the DERBY_LIB_DIR variable.
> On Linux the output is:
> $java -cp . DualBootRepro
> Loading derby from file:10.1.2.1/derby.jar
> 10.1.2.1/derby.jar
> Booted database in loader [EMAIL PROTECTED]
> FAIL: Booted database in 2nd loader [EMAIL PROTECTED]
> On Windows I get the expected output.
> $ java -cp . DualBootRepro
> Loading derby from file:10.1.2.1/derby.jar
> 10.1.2.1/derby.jar
> Booted database in loader [EMAIL PROTECTED]
> PASS: Expected exception for dualboot:Another instance of Derby may have
> already booted the database D:\marsden\repro\dualboot\mydb.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
