serhiy-bzhezytskyy commented on code in PR #4784:
URL: https://github.com/apache/solr/pull/4784#discussion_r3838235207
##########
solr/core/src/java/org/apache/solr/core/SolrCore.java:
##########
@@ -882,7 +864,14 @@ void initIndex(boolean passOnPreviousState, boolean
reload) throws IOException {
final String lockType = getSolrConfig().indexConfig.lockType;
Directory dir = directoryFactory.get(indexDir, DirContext.DEFAULT,
lockType);
try {
- if (isWriterLocked(dir)) {
Review Comment:
Correct, this inlines `isWriterLocked's body verbatim -- that's the intended
scope here, removing the deprecated wrapper without changing behavior. Checked
Lucene's actual `Directory`/`IndexWriter` API (pinned version 10.4.0, and
current trunk): `obtainLock(String)` is the only lock-related method, no
`objectLock` or similar exists.
The TOCTOU race the deprecation javadoc warns about is real -- reproduced it
directly: probe the write lock and release it (exactly what this inlined code
does), have another thread grab it in the gap, then try to acquire it again
like `initWriters()` does moments later (`SolrCore.java:1133-1134` ->
`DefaultSolrCoreState.createMainIndexWriter()`) -- the second acquisition fails
with `LockObtainFailedException` despite the probe having reported "clear". But
it's pre-existing and unrelated to this PR -- fixing it means redesigning that
two-step locking flow, not removing a deprecated method.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
