[jira] [Commented] (GEODE-6927) possible NPE in ConnectionTable.getThreadOwnedConnection if concurrent close

["ASF subversion and git services (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/GEODE-6927?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16993022#comment-16993022 ] 

ASF subversion and git services commented on GEODE-6927:
--------------------------------------------------------

Commit ff7883275827636ff495fcfb255df50691b66f13 in geode's branch refs/heads/develop from mkevo
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=ff78832 ]

GEODE-6927 make getThreadOwnedConnection code thread safe (#4085)

* GEODE-6927 make getThreadOwnedConnection code thread safe

* changes due to comments

* fix spotlessAply

* fix clean method

* remove null checks

* empty commit to re-trigger CI


> possible NPE in ConnectionTable.getThreadOwnedConnection if concurrent close
> ----------------------------------------------------------------------------
>
>                 Key: GEODE-6927
>                 URL: https://issues.apache.org/jira/browse/GEODE-6927
>             Project: Geode
>          Issue Type: Bug
>          Components: messaging
>            Reporter: Darrel Schneider
>            Assignee: Mario Kevo
>            Priority: Minor
>              Labels: easy-fix, needs-review, pull-request-available
>          Time Spent: 1h 50m
>  Remaining Estimate: 0h
>
> ConnectionTable.getThreadOwnedConnection has a couple of usages of "this.threadConnectionMap" that could result in an NPE if the ConnectionTable is concurrently closed.
> The unsafe code:
> {code:java}
>     if (this.threadConnectionMap == null) {
>       // This instance is being destroyed; fail the operation
>       closeCon(
>           "Connection table being destroyed",
>           result);
>       return null;
>     }
>     ArrayList al = (ArrayList) this.threadConnectionMap.get(id);
>     if (al == null) {
>       // First connection for this DistributedMember. Make sure list for this
>       // stub is created if it isn't already there.
>       al = new ArrayList();
>       // Since it's a concurrent map, we just try to put it and then
>       // return whichever we got.
>       Object o = this.threadConnectionMap.putIfAbsent(id, al);
>       if (o != null) {
>         al = (ArrayList) o;
>       }
>     }
> {code}
> If a close happens after the initial null check but before either the get or putIfAbsent call then a NPE will be thrown.
> All the other code that uses "threadConnectionMap" is careful to save it into a local var, and then use the local var. That is all that is needed here to make this code thread safe.



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