[ https://issues.apache.org/jira/browse/LUCENE-2421?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12863099#action_12863099 ]
Shai Erera commented on LUCENE-2421: ------------------------------------ You're right Mark ! I did not do a thorough check on NativeFSLock.obtain(). In the beginning of the method, it calls lockExists() and I assumed it checks for the existence of the file. But it actually checks whether the FileLock is not null. So that means: * If one obtains a Native lock and later a Simple lock obtain is attempted - it will fail. * If one obtains a Simple lock and later a Native lock obtain is attempted - it will succeed. I wrote the following code to demonstrate that: {code} SimpleFSLockFactory simple = new SimpleFSLockFactory(dir); NativeFSLockFactory nativel = new NativeFSLockFactory(dir); System.out.println(nativel.makeLock("test").obtain()); System.out.println(simple.makeLock("test").obtain()); {code} This prints "true" and "false" while if you move the simple.makeLock line above the native, it prints "true" twice. I don't know if that's a problem or not because it all boils down to whether we want to be nice if the user has made a mistake and used two lock factories in two different places of the code. Given that, if we are ok to declare this is unsupported in the sense that the code won't play nice, then I'm ok w/ not failing if the lock file deletion fails, for both the regular and test lock. I think it makes sense to decide the code doesn't play nice, because someone can anyway extend LF and do such silly mistakes ... > Hardening of NativeFSLock > ------------------------- > > Key: LUCENE-2421 > URL: https://issues.apache.org/jira/browse/LUCENE-2421 > Project: Lucene - Java > Issue Type: Improvement > Components: Index > Reporter: Shai Erera > Assignee: Shai Erera > Fix For: 3.1 > > Attachments: LUCENE-2421.patch, LUCENE-2421.patch > > > NativeFSLock create a test lock file which its name might collide w/ another > JVM that is running. Very unlikely, but still it happened a couple of times > already, since the tests were parallelized. This may result in a false > exception thrown from release(), when the lock file's delete() is called and > returns false, because the file does not exist (deleted by another JVM > already). In addition, release() should give a second attempt to delete() if > it fails, since the file may be held temporarily by another process (like > AntiVirus) before it fails. The proposed changes are: > 1) Use ManagementFactory.getRuntimeMXBean().getName() as part of the test > lock name (should include the process Id) > 2) In release(), if delete() fails, check if the file indeed exists. If it > is, let's attempt a re-delete() few ms later. > 3) If (3) still fails, throw an exception. Alternatively, we can attempt a > deleteOnExit. > I'll post a patch later today. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org