[jira] [Updated] (LUCENE-9306) The check against null and usage of the field updateThread is typically protected by synchronization, but not in 1 location.

["Adrian Nistor (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/LUCENE-9306?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Adrian Nistor updated LUCENE-9306:
----------------------------------
    Status: Patch Available  (was: Open)

> 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
>            Priority: Major
>         Attachments: LUCENE-9306.patch
>
>
> 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-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org