[jira] [Work logged] (HIVE-23841) Field writers is an HashSet, i.e., not thread-safe. Field writers is typically protected by synchronization on lock, but not in 1 location.

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/HIVE-23841?focusedWorklogId=458379&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-458379 ]

ASF GitHub Bot logged work on HIVE-23841:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 13/Jul/20 23:54
            Start Date: 13/Jul/20 23:54
    Worklog Time Spent: 10m 
      Work Description: adriannistor commented on pull request #1248:
URL: https://github.com/apache/hive/pull/1248#issuecomment-657887992


   I see this failure:
   
   http://ci.hive.apache.org/blue/organizations/jenkins/hive-precommit/detail/PR-1248/1/tests
   
   about `testExecuteStatementParallel – org.apache.hive.service.cli.TestEmbeddedThriftBinaryCLIService`
   
   But somehow this failure / test does not seem related to the change made.
   
   Let me know if you have any suggestions (e.g., maybe the fix needs 2 locks, both `INSTANCE`, which is really `this` and `lock` --- seems improbable).


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Issue Time Tracking
-------------------

            Worklog Id:     (was: 458379)
    Remaining Estimate: 0h
            Time Spent: 10m

> Field writers is an HashSet, i.e., not thread-safe.  Field writers is typically protected by synchronization on lock, but not in 1 location.
> --------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HIVE-23841
>                 URL: https://issues.apache.org/jira/browse/HIVE-23841
>             Project: Hive
>          Issue Type: Bug
>         Environment: Any environment
>            Reporter: Adrian Nistor
>            Priority: Major
>              Labels: patch-available
>         Attachments: HIVE-23841.patch
>
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> I also submitted a pull request on github at:
>  
> [https://github.com/apache/hive/pull/1248]
>  
> (same patch)
> h1. Description
>  
> Field {{writers}} is a {{HashSet}} ([line 70|https://github.com/apache/hive/blob/c93d7797329103d6c509bada68b6da7f907b3dee/ql/src/java/org/apache/hadoop/hive/llap/LlapOutputFormatService.java#L70]), i.e., not thread-safe.
> Accesses to field {{writers}} are protected by synchronization on {{lock}}, e.g., at lines: [141-144|https://github.com/apache/hive/blob/c93d7797329103d6c509bada68b6da7f907b3dee/ql/src/java/org/apache/hadoop/hive/llap/LlapOutputFormatService.java#L141-L144], [212-213|https://github.com/apache/hive/blob/c93d7797329103d6c509bada68b6da7f907b3dee/ql/src/java/org/apache/hadoop/hive/llap/LlapOutputFormatService.java#L212-L213], and [212-215|https://github.com/apache/hive/blob/c93d7797329103d6c509bada68b6da7f907b3dee/ql/src/java/org/apache/hadoop/hive/llap/LlapOutputFormatService.java#L212-L215].
> However, the {{writers.remove()}} at [line 249|https://github.com/apache/hive/blob/c93d7797329103d6c509bada68b6da7f907b3dee/ql/src/java/org/apache/hadoop/hive/llap/LlapOutputFormatService.java#L249] is protected by synchronization on {{INSTANCE}}, *not* on {{lock}}.
> Synchronizing on 2 different objects does not ensure mutual exclusion. This is because 2 threads synchronizing on different objects can still execute in parallel at the same time.
> Note that lines [215|https://github.com/apache/hive/blob/c93d7797329103d6c509bada68b6da7f907b3dee/ql/src/java/org/apache/hadoop/hive/llap/LlapOutputFormatService.java#L215] and [249|https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/llap/LlapOutputFormatService.java#L249] are modifying {{writers}} with {{put()}} and {{remove()}}, respectively.
> h1. The Code for This Fix
> This fix is very simple: just change {{synchronized (INSTANCE)}} to {{synchronized (lock)}}, just like the methods containing the other lines listed above.[]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)