Bug 20256 – problem with signal handling and parallel GC on linux

Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P1
Component
druntime
Product
D
Version
D2
Platform
x86
OS
Linux
Creation time
2019-09-30T14:27:48Z
Last change time
2019-10-06T10:18:05Z
Keywords
pull
Assigned to
No Owner
Creator
igor.khasilev

Comments

Comment #0 by igor.khasilev — 2019-09-30T14:27:48Z
Hello, Attached code fails with dmd 2.087 and 2.088 if parallel GC scanning enabled. import std.stdio; import core.sys.posix.signal; import core.thread; import core.sys.posix.unistd; import core.sys.posix.sys.wait; import core.memory; import std.algorithm; import std.range; void block(int signum) { sigset_t m; sigemptyset(&m); sigaddset(&m, signum); sigprocmask(SIG_BLOCK, &m, null); } void main() { auto x = new int[](10000); iota(10000).each!(i => x ~= i); GC.collect(); // ldc create thread here block(SIGHUP); // block works only for current thread, not for thread created by GC.collect auto parent_pid = getpid(); auto child_pid = fork(); if ( child_pid == 0 ) { //child kill(parent_pid, SIGHUP); // send signal to parent _exit(0); } if ( child_pid == -1 ) { _exit(0); } Thread.sleep(1.seconds); writeln("This should be printed"); } Expected output "This should be printed", instead program killed by signal SIGHUP. This code works as expected if program started with --DRT-gcopt=parallel:0 This is because there is no SIGHUP mask in sigmask of the GC stream (as if it were cloned after calling sigprocmask from the main stream). Problem would be fixed if scanner thread will have all signals blocked (except signals it have to handle).
Comment #1 by r.sagitario — 2019-09-30T17:43:54Z
I guess the same would happen with any other thread that has been created before forking. Not sure if this is something that can be solved in the runtime. It's been recently discussed to make parallel marking opt-in, so additional threads would not be created by default. See https://issues.dlang.org/show_bug.cgi?id=20219 You can also embed the runtime option into the binary, see https://dlang.org/spec/garbage.html#gc_config
Comment #2 by igor.khasilev — 2019-09-30T18:11:23Z
(In reply to Rainer Schuetze from comment #1) > I guess the same would happen with any other thread that has been created > before forking. Yes, but developer is aware of the situation if he create tread himself. And totally unaware and get unexpected program behavior if GC create thread. Or will not receive unexpected behaviour if GC decide to start scan thread later than sigprocmask were called. > Not sure if this is something that can be solved in the > runtime. In runtime this can be solved by masking all signals before spawing GC thread and restoring afterward. Something like (https://github.com/dlang/druntime/blob/master/src/gc/impl/conservative/gc.d#L2824): version(Linux) { sigset_t old_mask, new_mask; sigemptyset(&new_mask); sigaddset(&new_mask, SIGHUP); sigaddset(&new_mask, SIGINT); .... sigaddset(&new_mask, SIGTERM); ... sigprocmask(SIG_BLOCK, &new_mask, &old_mask); // block anything for scanBackgroung threads and store current mask } for (int idx = 0; idx < numScanThreads; idx++) scanThreadData[idx].tid = createLowLevelThread(&scanBackground, 0x4000, &stopScanThreads); version(Linux) { sigprocmask(SIG_UNBLOCK, &old_mask, null); } > > It's been recently discussed to make parallel marking opt-in, so additional > threads would not be created by default. See > https://issues.dlang.org/show_bug.cgi?id=20219 this probably will help. > > You can also embed the runtime option into the binary, see > https://dlang.org/spec/garbage.html#gc_config What if I am library author? I can't control this option, I'm just expect that sigmask in main thread works for the whole process.
Comment #3 by igor.khasilev — 2019-09-30T18:37:04Z
To be clear - I'm not against parallel GC, just against unexpected behavior. I can spend time and prepare PR if it hav any chance to be accepted.
Comment #4 by r.sagitario — 2019-10-01T05:46:06Z
(In reply to igor.khasilev from comment #3) > I can spend time and prepare PR if it hav any chance to be accepted. That would be great. I'm not a posix/linux expert, so help is appreciated.
Comment #5 by igor.khasilev — 2019-10-02T07:22:17Z
Comment #6 by dlang-bot — 2019-10-02T07:42:46Z
@ikod updated dlang/druntime pull request #2811 "fix issue 20256" fixing this issue: - fix issue 20256 - (linux) block signals in parallel gc scan thread - fix issue 20256 - (linux) block signals in parallel gc scan thread - fix issue 20256 https://github.com/dlang/druntime/pull/2811
Comment #7 by dlang-bot — 2019-10-05T07:07:54Z
@ikod created dlang/druntime pull request #2813 "problem with signal handling and parallel GC on linux" fixing this issue: - fix issue 20256 - problem with signal handling and parallel GC on linux - fix issue 20256 - problem with signal handling and parallel GC on linux https://github.com/dlang/druntime/pull/2813
Comment #8 by dlang-bot — 2019-10-06T10:18:05Z
dlang/druntime pull request #2813 "problem with signal handling and parallel GC on linux" was merged into stable: - 0217ac2af215f78e3a41bbbfa3ccc6f54f457aca by Igor Khasilev: fix issue 20256 - problem with signal handling and parallel GC on linux https://github.com/dlang/druntime/pull/2813