Fix racy iterator use in Node::AddConnection

Before this fix an iterator to `connections_` which requires a lock
would be dereferenced outside an unlock operation because the `it` taken
from the map isn't understood as guarded by the same lock.

This takes a Ref<NodeLink> before unlocking which'll keep the link
reference alive even if `connections_` is concurrently modified and the
entry removed (or replaced).

Bug: 1523704
Change-Id: I6f6fe4e34ec2c8268d4e7f33965a13e3b10f9f92
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5254025
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: Ken Rockot <rockot@google.com>
Auto-Submit: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1254709}

third_party/ipcz/src/ipcz/node.cc

@@ -169,9 +169,10 @@ bool Node::AddConnection(const NodeName& remote_node_name,
       // handling an incoming NodeConnector message, we can err on the side of
       // caution (i.e. less re-entrancy in event handlers) by treating every
       // case like an API call.
+      const Ref<NodeLink> link = it->second.link;
       mutex_.Unlock();
       const OperationContext context{OperationContext::kAPICall};
-      DropConnection(context, *it->second.link);
+      DropConnection(context, *link);
       mutex_.Lock();
     }