If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Make debugging on Linux work again

RESOLVED FIXED in mozilla26

Status

()

Core
IPC
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: evilpie, Assigned: billm)

Tracking

unspecified
mozilla26
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 786570 [details] [diff] [review]
setgpid

For some reason we need to call setpgid in the child. Bill made this change on larch. Without this patch setting a breakpoint in gdb is impossible, because the browser crashes after continuing execution. I will leave it Bill to explain this.
(Assignee)

Comment 1

4 years ago
It seems like this probably isn't the right thing to do, but I don't know what's right.

The problem is that, when you hit ^C, all processes in the foreground process group receive the SIGINT. If you hit ^C in gdb, gdb normally traps the signal and instead suspends the debuggee. However, it only does this for the process being debugged. So child processes will still get the signal (since they're in the same process group as the parent) and they'll get killed.

The patch just puts all child processes in their own process group. This is good for gdb. However, it means that when you're running FF outside a debugger, hitting ^C to kill won't actually kill the children. In reality they'll probably die off soon after the parent dies, but it still seems kind of bad.

Maybe bsmedberg will have an idea what to do here.
Flags: needinfo?(benjamin)

Comment 2

4 years ago
You could just trap and ignore SIGINT in child processes. Note that IPC channels should notice when the other side closes, fire ActorDestroyed on the root actor, and that should abort the child process.
Flags: needinfo?(benjamin)
(Assignee)

Comment 3

4 years ago
The problem is that we also want to be able to debug the child process while the parent is running. In that case we don't want ^C to kill the parent. And we don't want to ignore SIGINT in the parent.

Maybe I'll try to figure out what chrome does.

Comment 4

4 years ago
I'm confused, I think. If you're debugging a child process, your gdb should be running in a separate terminal session, right? So control-c in that session shouldn't affect either the child or the parent.

I was only suggesting to ignore SIGINT in child processes, not the parent process, which should be possible by switching on XRE_GetProcessType
(Assignee)

Comment 5

4 years ago
Oh, I see, you're right. I forgot that child processes will get debugged in a different terminal.
(Assignee)

Comment 6

4 years ago
Created attachment 787246 [details] [diff] [review]
signal-handler

This does as suggested.
Attachment #786570 - Attachment is obsolete: true
Attachment #787246 - Flags: review?(benjamin)
(Assignee)

Comment 7

4 years ago
Comment on attachment 787246 [details] [diff] [review]
signal-handler

Ted, can you review this?
Attachment #787246 - Flags: review?(benjamin) → review?(ted)
Comment on attachment 787246 [details] [diff] [review]
signal-handler

Review of attachment 787246 [details] [diff] [review]:
-----------------------------------------------------------------

I was a little wary of side-effects of this, but I suspect SIGINT isn't useful for much else so ignoring it shouldn't be terribly harmful. I doubt anyone really expects to be able to Ctrl+C kill non-foreground processes anyway.
Attachment #787246 - Flags: review?(ted) → review+
(Assignee)

Comment 9

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c37719f4d482
https://hg.mozilla.org/mozilla-central/rev/c37719f4d482
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.