[GEODE-6927] possible NPE in ConnectionTable.getThreadOwnedConnection if concurrent close

• Type: Bug
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Fix Version/s: 1.12.0
• Component/s: messaging
• Labels: easy-fix needs-review pull-request-available
• Assignee: Ernest Burghardt
• Reporter: Darrel Schneider
• Created: 28/Jun/19 17:34
• Updated: 30/Mar/20 23:32
• Resolved: 11/Dec/19 08:09

Description

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:

    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;
      }
    }

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.

Issue Links

• links to: GitHub Pull Request #4085