-
Notifications
You must be signed in to change notification settings - Fork 100
Fix race between parent and monitor in use_pty
#1130
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
base: main
Are you sure you want to change the base?
Conversation
use_pty
By the way, I have left b156eb9 in for illustration purposes only. That fix alone takes care of the problem "in practice". We can drop that one before it is merged. |
Note, another solution would maybe be to not |
This fixes the problem on the FreeBSD machine I tested, but not on the Debian trixie machine. |
The problem with #1129 was that when the monitor shuts down, it takes the pseudoterminal it controls with it. So we end up in a condition where the parent is racing with the monitor to read remaining data before the monitor crosses the finish line.
This could be demonstrated by adding a sleep in the monitor, but a better fix is simply to use the existing backchannel to synchronise parent and monitor. I re-used the old
ExecCommand
message for this, as that was already used to handle a similar race at the start.So basically the backchannel protocol is: parent sends an
Edge
to signal the monitor it can go (this was already the case), do the usual thing while the monitor and child are doing their dance, and then again when the monitor has communicated that it is ended, let it wait for anotherEdge
from the parent.NOTE: This fixes #1129 on FreeBSD, but not on the Trixie machine exhibiting the behaviour.
Haven't been able to reproduce on a second machine reliably.