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

mtest: limit "magic" CTRL+C behavior to process group leaders #9410

Merged
merged 1 commit into from Oct 18, 2021

Conversation

bonzini
Copy link
Contributor

@bonzini bonzini commented Oct 18, 2021

If meson is not a process group leader, a SIGINT will be delivered also to its parent process (and possibly other processes). The parent process then will probably exit and mtest will continue running in the background, without any way to interrupt the run completely.

To fix this, treat SIGINT and SIGTERM the same way unless mtest is a process group leader.

This can be reproduced when a parallel make invokes "meson test".

If meson is not a process group leader, a SIGINT will be delivered also to
its parent process (and possibly other processes).  The parent process then
will probably exit and mtest will continue running in the background, without
any way to interrupt the run completely.

To fix this, treat SIGINT and SIGTERM the same way unless mtest is a
process group leader.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
@dcbaker dcbaker added this to the 0.60.0 milestone Oct 18, 2021
@codecov
Copy link

codecov bot commented Oct 18, 2021

Codecov Report

Merging #9410 (03cb208) into master (6718556) will decrease coverage by 0.00%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9410      +/-   ##
==========================================
- Coverage   67.26%   67.26%   -0.01%     
==========================================
  Files         396      396              
  Lines       85388    85392       +4     
  Branches    17477    17479       +2     
==========================================
- Hits        57440    57439       -1     
- Misses      23252    23255       +3     
- Partials     4696     4698       +2     
Impacted Files Coverage Δ
mesonbuild/mtest.py 77.65% <33.33%> (-0.12%) ⬇️
mesonbuild/scripts/vcstagger.py 87.50% <0.00%> (-4.17%) ⬇️
mtest.py 76.14% <0.00%> (-0.11%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6718556...03cb208. Read the comment docs.

@xclaesse
Copy link
Member

Seems to make sense. OOC, when is meson not the group leader? You wrote a scrip that does that specifically?

@bonzini
Copy link
Contributor Author

bonzini commented Oct 18, 2021

when is meson not the group leader

The shell's job control functionality places each process started by the shell in a separate process group, but otherwise Meson is never the group leader. In particular, it is not a group leader if invoked by a program other than the shell, or if the shell does not perform job control (as is the case when running a shell script).

@bonzini
Copy link
Contributor Author

bonzini commented Oct 18, 2021

(In fact you can see from the coverage report that Meson is never a group leader during a CI run).

@xclaesse xclaesse merged commit 8945c53 into mesonbuild:master Oct 18, 2021
@xclaesse
Copy link
Member

Ok, LGTM.

@nirbheek nirbheek modified the milestones: 0.60.0, 0.59.3 Oct 19, 2021
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.

None yet

4 participants