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

Adrian Nistor updated LUCENE-9306:
----------------------------------
    Lucene Fields: New,Patch Available  (was: New)

> 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
>          Components: modules/replicator
>            Reporter: Adrian Nistor
>            Priority: Major
>         Attachments: LUCENE-9306.patch
>
>
> h1. Github Pull Request
> I created a pull request on github for this issue at:
> [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

Reply via email to