The check against null and usage of the field updateThread is typically protected by synchronization, but not in 1 location. #1413
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Report on JIRA
I also reported this issues on JIRA at:
https://issues.apache.org/jira/browse/LUCENE-9306
Bug Description
Checks against
null
and usages of the fieldupdateThread
are typically made atomic by synchronization onthis
(synchronized methods), e.g., at lines: 339-341, 357-358, and 380-381.However, the
null
check at line 387 and the usage at line 388 are not protected bysynchronized
.This check against
null
and usage ofupdateThread
are intoString()
.However, the problem is not that the
toString()
will give a garbled string (i.e., a relatively minor issues).The problem is that, in between the
null
check and the usage,updateThread
can be set tonull
(e.g., by line 369) and therefore the code can crash.I.e., without
synchronized
, thenull
check does not protect theupdateThread.getName()
usage.I don't know how
toString()
is called concurrently. However, it seems like a dangerous assumption to make that the callers oftoString()
know it should not be called concurrently with line 369, especially as the other methods do protect with synchronization thenull
check and usage.This Patch's Code
The fix is very simple: just make the method containing lines 387-388
synchronized
, just like the other methods containingnull
checks and usages ofupdateThread
.