Skip to content

Conversation

RainbowSimon
Copy link
Contributor

Contribution description

When 2 posix TCP sockets in different threads are open and the first one is in accept() the mutex of the socket pool was not unlocked until data was received. When another thread calls a function which needs the socket pool mutex there was a deadlock. Unlocking the mutex after all operations on the socket pool are done, in particular BEFORE sock_tcp_accept() solves this.

I verified all function calls between my insertion and the later mutex unlock do not access the socket pool.

Testing procedure

Since this change is small I hope it is sufficient to describe the problem:

In one thread open a posix socket and accept(), this blocks until data is received.
In another thread call any other socket pool dependent posix/socket function, like socket().

Expected:
Second thread can create/... sockets.

Actual:
Deadlock, as the socket pool is still locked.

Issues/PRs references

None, just found this without creating an issue, while porting posix dependent code.

When 2 posix TCP sockets in different threads are open and the first
one is in accept() the mutex of the socket pool was not unlocked
until data was received. When another thread calls a function which
needs the socket pool mutex there was a deadlock. Unlocking the mutex
after all operations on the socket pool are done, in particular BEFORE
sock_tcp_accept() solves this.
@github-actions github-actions bot added the Area: sys Area: System label Oct 11, 2025
@crasbe crasbe added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: ready for merge train 🚃 PR is ready to be merged and awaiting the next merge train labels Oct 13, 2025
@riot-ci
Copy link

riot-ci commented Oct 13, 2025

Murdock results

✔️ PASSED

0fa8104 posix/sockets: Prevent deadlock with multiple TCP sockets

Success Failures Total Runtime
10516 0 10516 30m:33s

Artifacts

@crasbe crasbe removed the CI: ready for merge train 🚃 PR is ready to be merged and awaiting the next merge train label Oct 13, 2025
@crasbe
Copy link
Contributor

crasbe commented Oct 14, 2025

With your proposed solution, wouldn't the mutex be double-freed in line 578 if everything goes successfully?
When two threads are running, this could accidentally free the mutex from another thread if I understand it correctly. Of course the timing has to be just right for that to happen, but it looks like a race condition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants