Fix crash when BRBitcoinPeer disconnects fast #410
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dear Blockset maintainers,
I was playing around with the great functionality of walletkit, but then I stumbled upon a strange crash.
The crash happened especially often when trying to connect to a BitcoinPeer in iPhone-airplane-mode, but it also sometimes happened when my iPhone was not in airplane-mode.
My investigations made it clear that I was dealing with a race-condition and a potential use-after-free.
Therefore, this PR fixes a suspected race-condition when a BRBitcoinPeer disconnects within a very short time.
I don't have a minimum reproducible sample, but I will try to explain how I discovered this crash:
It starts with connecting to BitCoin-peers in peer-2-peer-only-mode, when we reach
btcPeerConnect
withinBRBitcoinPeer.c
.btcPeerConnect
spawns a new thread that executes_peerThreadRoutine
._peerThreadRoutine
tries to establish a socket, when it fails to establish a socket, then it will eventually call_peerDisconnected
and exit the previously created thread.An "info-struct" gets passed to
_peerDisconnected
._peerDisconnected
frees members of the info-struct and does some cleanup-stuff.However, I suspect that
_peerDisconnected
might be called twice for the same info-struct.Let me show you the two callsites of
_peerDisconnected
.The first callsite is the following code-snippet, which has a suspicious locking because
manager->lock
gets unlocked and then a few lines later locked again within_peerDisconnected
.The second callsite is a threadCleanup-function of
_peerThreadRoutine
, as shown in the following code-snippet:Now my suspection is that those callsites are racing against each other.
In particular, I observed that
_peerDisconnected
got invoked with a garbage-info-struct that led to segmentation-faults withinbtcPeerFree
.Can you confirm that there are troubles with the multithreaded cleanup of BRBitcoinPeers?