Adrian Nistor created LUCENE-9306:
-------------------------------------
Summary: The check against null and usage of the field
updateThread is typically protected by synchronization, but not in 1 location.
Key: LUCENE-9306
URL: https://issues.apache.org/jira/browse/LUCENE-9306
Project: Lucene - Core
Issue Type: Bug
Reporter: Adrian Nistor
h1. Github Pull Request
I created a pull request on github for this:
https://github.com/apache/lucene-solr/pull/1413
h1. Description
Checks against {{null}} and usages of the field {{updateThread}} are typically
made atomic by synchronization on {{this}} (synchronized methods), e.g., at
lines:
[339-341|https://github.com/apache/lucene-solr/blob/531015245042507d71845b6d584e7e7389303093/lucene/replicator/src/java/org/apache/lucene/replicator/ReplicationClient.java#L339-L341],
[357-358|https://github.com/apache/lucene-solr/blob/531015245042507d71845b6d584e7e7389303093/lucene/replicator/src/java/org/apache/lucene/replicator/ReplicationClient.java#L357-L358],
and
[380-381|https://github.com/apache/lucene-solr/blob/531015245042507d71845b6d584e7e7389303093/lucene/replicator/src/java/org/apache/lucene/replicator/ReplicationClient.java#L380-L381].
However, the {{null}} check at [line
387|https://github.com/apache/lucene-solr/blob/531015245042507d71845b6d584e7e7389303093/lucene/replicator/src/java/org/apache/lucene/replicator/ReplicationClient.java#L387]
and the usage at [line
388|https://github.com/apache/lucene-solr/blob/531015245042507d71845b6d584e7e7389303093/lucene/replicator/src/java/org/apache/lucene/replicator/ReplicationClient.java#L388]
are not protected by {{synchronized}}.
{code:java}
public String toString() {
String res = "ReplicationClient";
if (updateThread != null) { <<<<<<<<<<<<<<<<<<<<<
res += " (" + updateThread.getName() + ")"; <<<<<<<<<<<<<<<<<
{code}
This check against {{null}} and usage of {{updateThread}} are in {{toString()}}.
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 to {{null}} (e.g., by [line
369|https://github.com/apache/lucene-solr/blob/531015245042507d71845b6d584e7e7389303093/lucene/replicator/src/java/org/apache/lucene/replicator/ReplicationClient.java#L369])
and therefore the code can crash.
I.e., without {{synchronized}}, the {{null}} check does not protect the
{{updateThread.getName()}} usage.
I don't know how {{toString()}} is called concurrently. However, it seems like
a dangerous assumption to make that the callers of {{toString()}} know it
should not be called concurrently with [line
369|https://github.com/apache/lucene-solr/blob/531015245042507d71845b6d584e7e7389303093/lucene/replicator/src/java/org/apache/lucene/replicator/ReplicationClient.java#L369],
especially as the other methods do protect with synchronization the {{null}}
check and usage.
h1. This Patch's Code
The fix is very simple: just make the method containing [lines
387-388|https://github.com/apache/lucene-solr/blob/531015245042507d71845b6d584e7e7389303093/lucene/replicator/src/java/org/apache/lucene/replicator/ReplicationClient.java#L387-L388]
{{synchronized}}, just like the other methods containing {{null}} checks and
usages of {{updateThread}}.
{code:java}
public synchronized String toString() { <<<<<<<<<<<<<< added "synchronized" here
String res = "ReplicationClient";
if (updateThread != null) {
res += " (" + updateThread.getName() + ")";
{code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
