[hibernate-issues] [Hibernate-JIRA] Commented: (HSEARCH-211) SharedReaderProvider: should own lock when testing for index state

[ http://opensource.atlassian.com/projects/hibernate/browse/HSEARCH-211?pag... ] Sanne Grinovero commented on HSEARCH-211: ----------------------------------------- Hello, please I could use some help about understanding the code of SharedReaderProvider, because my most impulsive solution would be to rewrite most of it which is obviously a bad idea. "directoryProviderLock.lock(); //needed for same problem as the double-checked locking" I don't understand the comment, to correctly save you from the double-checked locking issue I think you shouldn't release the lock before assigning a new value to the reader, otherwise it would be possible to initialize the values multiple times; what's the double-checking problem? If I keep the lock longer it will be held even around the replaceActiveReader(...) and because of the issue we would need to hold the lock during both branches of the "if (reader ==null)" Also because of this issue we could be trying to reopen an already closed reader if the lock is not held during the whole else branch, so it turns out we need to hold the lock during the whole iteration of the for cycle, without releasing it until next iteration. This would make the lock acquisition during replaceActiveReader useless, removing the need for the locks on semaphores as we are holding already wider-scope locks. Having this longer locks won't necessarily hurt performance, as they would also stop different threads from doing the same expensive operations twice (or more) such as testing index state and opening new readers. There's a note about scalability, IMHO having a second thread waiting for the first to finish the same job instead of having to do the same stuff is usually better. if the semaphoreLocks are needed, then: semaphoreIndexReaderLock is used to protect a HashMap (which could use a ConcurrentHashMap) and at the same time to protect readerSemaphores, even different semaphores are not permitted to be updated concurrently; it look like too restrictive, a per-dir lock could be used or the semaphores could use the real semaphore class instead of int, or the AtomicInteger. at last, the locks are currently not guaranteed to be visible to all threads: same care as already discussed for HSEARCH-189 is needed. Any comments about how I should proceed? IMHO I'll put the broader locks, I think they are needed. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.... - For more information on JIRA, see: http://www.atlassian.com/software/jira