Skip to content

conn: fix Receive blocking Send#291

Open
nickgarlis wants to merge 1 commit intomdlayher:mainfrom
nickgarlis:fix-blocked-send
Open

conn: fix Receive blocking Send#291
nickgarlis wants to merge 1 commit intomdlayher:mainfrom
nickgarlis:fix-blocked-send

Conversation

@nickgarlis
Copy link
Copy Markdown
Collaborator

In #265, Receive was changed from RLock to Lock to serialize concurrent Receive calls. Since Send uses RLock, it blocks for as long as Receive holds the exclusive lock. When Receive is blocking in recvmsg waiting for kernel data, Send is blocked too. This causes a deadlock in patterns like NFQUEUE, where one can goroutine read packets while another sends verdicts concurrently.

Fix by introducing a separate receiveMu that serializes concurrent Receive calls without holding mu exclusively, keeping Send and Receive concurrency compatible.

Fixes: #288

In mdlayher#265, Receive was changed from RLock to Lock to serialize concurrent
Receive calls. Since Send uses RLock, it blocks for as long as Receive
holds the exclusive lock. When Receive is blocking in recvmsg waiting for
kernel data, Send is blocked too. This causes a deadlock in patterns like
NFQUEUE, where one can goroutine read packets while another sends verdicts
concurrently.

Fix by introducing a separate receiveMu that serializes concurrent
Receive calls without holding mu exclusively, keeping Send and Receive
concurrency compatible.

Fixes: mdlayher#288
@nickgarlis
Copy link
Copy Markdown
Collaborator Author

@florianl Do you mind having a look ?

Copy link
Copy Markdown
Contributor

@florianl florianl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a test like https://gist.github.com/florianl/13bedaf2bb26bb3c0a0ce52df9e229fb that does not depend on time.Sleep()?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can no longer handle NFQUEUE reads from different goroutine than sending verdicts in v1.11

2 participants