Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore SIGPIPE in the Merlin server process #49

Merged
merged 1 commit into from
Apr 3, 2024
Merged

Conversation

catern
Copy link
Contributor

@catern catern commented Apr 3, 2024

The merlin server writes to pipes received from the merlin client process. If the read end of those pipes is closed, e.g. because the merlin client process was killed, then when the server writes to the pipe it will generate a SIGPIPE. This kills the Merlin server. Ignore the SIGPIPE instead of unnecessarily dying.

The fact that you get SIGPIPE when you write to a closed pipe is a classic Unix footgun, and this is the usual resolution. Another solution is to avoid pipes (in favor of e.g. socketpairs), but we can't do that because we write directly to the file descriptors received from the client.

The merlin server writes to pipes received from the merlin client process.  If the read
end of those pipes is closed, e.g. because the merlin client process was killed, then when
the server writes to the pipe it will generate a SIGPIPE.  This kills the Merlin server.
Ignore the SIGPIPE instead of unnecessarily dying.

The fact that you get SIGPIPE when you write to a closed pipe is a classic Unix footgun,
and this is the usual resolution.  Another solution is to avoid pipes (in favor of
e.g. socketpairs), but we can't do that because we write directly to the file descriptors
received from the client.
Copy link
Contributor

@ncik-roberts ncik-roberts left a comment

Choose a reason for hiding this comment

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

LGTM.

@ncik-roberts ncik-roberts merged commit fc2845a into main Apr 3, 2024
1 of 2 checks passed
@ncik-roberts ncik-roberts deleted the no-sigpipe branch April 3, 2024 18:33
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.

2 participants