cshannon commented on PR #3175: URL: https://github.com/apache/accumulo/pull/3175#issuecomment-1415831192
Something else I noticed is that it would be good to change this line here even though it's not related to the PR. https://github.com/apache/accumulo/blob/49d335313f18275a66e604e274db9ba6895a4373/core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/lru/SynchronousLoadingBlockCache.java#L116-L118 The call to lock() should always be outside of the try block so it should be: ```java loadLock.lock(); try { ... ``` This is because if the call to lock() fails you can end up with an IllegalMonitorStateException when the thread calls unlock() in the finally block since it doesn't own the lock. In practice this would be pretty rare as lock() _shouldn't_ ever really throw an exception. It's much more of an issue if you use something like `lockInterruptibly()` but it's still best practice to have it outside the try block. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
