fix suspend in session master when fork_on_start #296
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n_start When ui.sessions.fork_on_start, we fork a process that has a different fdout than get_output_handle() (which has hardcoded fdout of 1), so it will never be considered term->master, yet this is the process we want to wake up in SIGTSTP handler. Since we cannot rely on term->master to determine if we are the master process, we instead move master_pid to be set explicitly at the places where we have information about whether our process is a master or a slave: first on start, then once the interlink determination has been made. master_pid has to be set in both parent and child, because both will get suspended and need to know which one needs to resume in background with SIGCONT (the master). We can't inherit from the parent because it's unknown at the time of fork. Previously, master_pid worked correctly with fork_on_start=0, -no-connect or -dump, but not with fork_on_start=1. See rkd77#287 for background.
|
Also, what systems don't |
|
Just to clarify, slaves, and |
|
Seems to be good. Thanks |
|
I think it probably doesn't work right without HAVE_GETPID, but I don't know what system I can use to test it? Should we consider removing HAVE_GETPID and just breaking on systems without it? Does Windows define this? I don't have such a system available to check. |
|
DJGPP has getpid() also MINGW has it, so check for getpid() was removed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
After commit b102add following request #287,
term->masteris used as test to see if this is session master process we want to wakeup with SIGCONT after suspend (master must be kept running, or other slaves will block). Test uses comparisonterm->fdout == 1, which is never true in the case of forked master whenui.session.fork_on_start. The forked master has different (>10) fdout descriptor numbers, so it will never beterm->masterand thusmaster_pidwill still have its initialized value 0.Instead we can get the pid directly after forking, and use the interlink creation attempt to decide whether this is a master or slave process depending on successful connect(). This was tested with both
fork_on_start=1andfork_on_start=0with different suspend/resume permutations and seems to work correctly now.Note that
term->masteris used in several places:exec_on_terminal(),try_to_draw_images(),redraw_screen(),erase_screen(),get_resource_info()andget_master_session(), so it could be thatfork_on_starthas other issues.