Re: kcrash, fork, and stdout/stderr
On Tue, Dec 05, 2017 at 01:12:30PM +0100, David Faure wrote: > On lundi 4 décembre 2017 17:04:25 CET Thiago Macieira wrote: > > forking inside a signal handler is a bad idea because it may deadlock. If > > the crash happens while glibc holds some mutexes relating to > > pthread_atfork, then you'll have a problem. > > I see. But how should one implement a crash handler that autorestarts an app, > then, in a "standalone application" use case, i.e. no kdeinit or other daemon > running in the background? > you don't ... the fail-safe solution would be forking right after start, basically having a privately owned mini-kdeinit. but that seems like major overkill. the crash handler (and even more auto-restart) are nice-to-have features, so it seems reasonable that they don't work reliably outside a kde session.
Re: kcrash, fork, and stdout/stderr
On Sunday, 3 December 2017 02:54:21 PST David Faure wrote: > I'm trying to fix a bug (caught by CI) where KCrash, with the AutoRestart > flag, restarts the crashing process directly (fork+execve) rather than via > kdeinit. > > The first process then exits, and whenever the child writes to stderr, it > gets a SIGPIPE. Oops. > > How do I set things up so that the child's stderr is basically what the > parent's stderr was, in a way that works even if the parent exits? That's the default. You have to figure out what you've done to change that. You didn't say you're using QProcess. [continued on the other email] -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center
Re: kcrash, fork, and stdout/stderr
On mardi 5 décembre 2017 16:46:50 CET Thiago Macieira wrote: > On Tuesday, 5 December 2017 04:12:30 PST David Faure wrote: > > I see. But how should one implement a crash handler that autorestarts an > > app, then, in a "standalone application" use case, i.e. no kdeinit or > > other > > daemon running in the background? > > Wait, why are you forking in the first place? > > Just exec the process again. It will replace the current process without > closing the pipes. The parent won't notice a thing. Interesting idea. It might work, except that what KCrash currently does is both restart the app and attach drkonqi to the crashed app (or dump core, if drkonqi is disabled), so that users/developers know a crash happened. Doing just execve() wouldn't allow that. I'm tempted to just not touch it further. > Of course, that may be a problem: the parent may see the output from the > child process again. It would probably not matter for GUI apps, and the unittest could be adjusted. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: kcrash, fork, and stdout/stderr
On Tuesday, 5 December 2017 04:12:30 PST David Faure wrote:
> > forking inside a signal handler is a bad idea because it may deadlock. If
> > the crash happens while glibc holds some mutexes relating to
> > pthread_atfork, then you'll have a problem.
>
> I see. But how should one implement a crash handler that autorestarts an
> app, then, in a "standalone application" use case, i.e. no kdeinit or other
> daemon running in the background?
Use syscall(SYS_clone) instead of fork(). The system call always works, it's
glibc's fork() internals that are known to have problems.
Linux-only, of course.
>From forkfd.c:
/* start the child - can't use fork() because it may deadlock */
#if defined(__NR_clone2)
child_pid = syscall(__NR_clone2, SIGCHLD, NULL, 0, NULL, NULL, NULL);
#elif defined(__cris__) || defined(__s390__)
child_pid = syscall(__NR_clone, NULL, SIGCHLD, NULL, NULL, 0L);
#else
child_pid = syscall(__NR_clone, SIGCHLD, NULL, NULL, NULL, 0L);
#endif
if (child_pid == 0) {
/* child process - restore state */
--
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
Software Architect - Intel Open Source Technology Center
Re: kcrash, fork, and stdout/stderr
On Tuesday, 5 December 2017 04:12:30 PST David Faure wrote: > I see. But how should one implement a crash handler that autorestarts an > app, then, in a "standalone application" use case, i.e. no kdeinit or other > daemon running in the background? Wait, why are you forking in the first place? Just exec the process again. It will replace the current process without closing the pipes. The parent won't notice a thing. Of course, that may be a problem: the parent may see the output from the child process again. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center
Re: kcrash, fork, and stdout/stderr
On lundi 4 décembre 2017 17:04:25 CET Thiago Macieira wrote:
> On Monday, 4 December 2017 00:26:55 PST David Faure wrote:
> > > Or do you fork a child at that point? fork from inside a signal handler
> > > is
> > > an incredibly bad idea, don't do it.
> >
> > Well, I guess that's why it's the fallback from the main strategy which is
> > "ask kdeinit to restart that application" (even if it doesn't provide a
> > kdeinit module). But kdeinit might not be running, outside plasma
> > workspace.
> >
> > This has always been like that, it goes back to 2006 (Luboš), it was
> > discussed with you
> > https://marc.info/?l=kde-core-devel=113699766611213=2 ("There's still
> > a
> > fallback to use fork() in case using kdeinit fails for any reason.")
>
> Well, I've learnt a lot in the last 11 years.
>
> forking inside a signal handler is a bad idea because it may deadlock. If
> the crash happens while glibc holds some mutexes relating to
> pthread_atfork, then you'll have a problem.
I see. But how should one implement a crash handler that autorestarts an app,
then, in a "standalone application" use case, i.e. no kdeinit or other daemon
running in the background?
--
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5
Re: kcrash, fork, and stdout/stderr
On Monday, 4 December 2017 00:26:55 PST David Faure wrote:
> > Or do you fork a child at that point? fork from inside a signal handler is
> > an incredibly bad idea, don't do it.
>
> Well, I guess that's why it's the fallback from the main strategy which is
> "ask kdeinit to restart that application" (even if it doesn't provide a
> kdeinit module). But kdeinit might not be running, outside plasma workspace.
>
> This has always been like that, it goes back to 2006 (Luboš), it was
> discussed with you
> https://marc.info/?l=kde-core-devel=113699766611213=2 ("There's still a
> fallback to use fork() in case using kdeinit fails for any reason.")
Well, I've learnt a lot in the last 11 years.
forking inside a signal handler is a bad idea because it may deadlock. If the
crash happens while glibc holds some mutexes relating to pthread_atfork, then
you'll have a problem.
--
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
Software Architect - Intel Open Source Technology Center
Re: kcrash, fork, and stdout/stderr
On lundi 4 décembre 2017 05:43:14 CET Thiago Macieira wrote:
> > Yes, no kdeinit involved. Here's the setup:
> >
> > $ bin/kcrashtest testAutoRestartDirectly
> >
> > starts test_crasher (with QProcess, using waitForFinished())
> >
> > which crashes :)
> > KCrash catches that, and since the Autorestart flag is set,
> >
> > sets the env var KCRASH_AUTO_RESTARTED,
> > and starts test_crasher again (it checks that env var, so no
> >
> > infinite loop)
> >
> > The first test_crasher then exits (after waitpid(), but that's just
> > waiting
> > for the child to start, right?).
>
> Sorry, more details here. Can you provide the strace for clone, pipe, pipe2,
> dup, dup2, dup3, execve, waitpid, waitid, wait4?
>
> QProcess starts a process.
>
> Does that process start something else? Or is QProcess's direct child that
> crashes?
It's exactly as I described. It's QProcess's direct child which crashes
(and which has a segv handler enabled, KCrash).
> The crash handler is in-process, correct? Then you execve in place?
fork+execve
https://lxr.kde.org/source/frameworks/kcrash/src/kcrash.cpp#0733
> Or do you fork a child at that point? fork from inside a signal handler is
> an incredibly bad idea, don't do it.
Well, I guess that's why it's the fallback from the main strategy which is
"ask kdeinit to restart that application" (even if it doesn't provide a
kdeinit module). But kdeinit might not be running, outside plasma workspace.
This has always been like that, it goes back to 2006 (Luboš), it was discussed
with you https://marc.info/?l=kde-core-devel=113699766611213=2
("There's still a fallback to use fork() in case using kdeinit fails for
any reason.")
> That makes QProcess not create a pipe and instead pass its own stderr to the
> child. Then it's SEP.
Yep, so from my POV the problem is fixed.
Thanks for the additional input!
--
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5
Re: kcrash, fork, and stdout/stderr
On Sunday, 3 December 2017 12:12:53 PST David Faure wrote: > > This should already work, unless the O_CLOEXEC flag has been set on the > > file descriptor for stderr. The kernel should duplicate open > > descriptors on fork(2), and exec(2) should maintain its association to > > any already-open descriptors. > > Hmm then I don't understand the SIGPIPE. SIGPIPE is sent to the writing process whenever the write would end in EPIPE. That's a legacy of Unix that makes sense for command-line applications that should exit if the downstream pipeline closed, but in non-graphical applications it makes no sense. It makes zero sense to do that for sockets... I have a pending Linux kernel patch to disable that per file descriptor, following OpenBSD's API. Anyway, moving on... > > But, which process was the one writing to the terminal? Do we know that > > the destination of stderr was a PTY instead of being filtered through > > some kind of kdeinit magic? > > Yes, no kdeinit involved. Here's the setup: > > $ bin/kcrashtest testAutoRestartDirectly > starts test_crasher (with QProcess, using waitForFinished()) > which crashes :) > KCrash catches that, and since the Autorestart flag is set, > sets the env var KCRASH_AUTO_RESTARTED, > and starts test_crasher again (it checks that env var, so no > infinite loop) > > The first test_crasher then exits (after waitpid(), but that's just waiting > for the child to start, right?). Sorry, more details here. Can you provide the strace for clone, pipe, pipe2, dup, dup2, dup3, execve, waitpid, waitid, wait4? QProcess starts a process. Does that process start something else? Or is QProcess's direct child that crashes? The crash handler is in-process, correct? Then you execve in place? Or do you fork a child at that point? fork from inside a signal handler is an incredibly bad idea, don't do it. Or does the signal handler write to another process which in turn restarts the process? > Is this because QProcess set up something for catching stderr, which no > longer exists when the process exits? Possibly. QProcess creates a pipe for stderr and it exists only so long as the direct child exists. Once the child exits, QProcess closes the reading end of the pipe and any process that tries to write to it will get SIGPIPE. > Oh. > proc.setProcessChannelMode(QProcess::ForwardedChannels); > fixes it! That makes QProcess not create a pipe and instead pass its own stderr to the child. Then it's SEP. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center
Re: kcrash, fork, and stdout/stderr
On dimanche 3 décembre 2017 17:17:10 CET Michael Pyne wrote: > On Sun, Dec 03, 2017 at 11:54:21AM +0100, David Faure wrote: > > I'm trying to fix a bug (caught by CI) where KCrash, with the AutoRestart > > flag, restarts the crashing process directly (fork+execve) rather than > > via kdeinit. > > > > The first process then exits, and whenever the child writes to stderr, it > > gets a SIGPIPE. Oops. > > > > How do I set things up so that the child's stderr is basically what the > > parent's stderr was, in a way that works even if the parent exits? > > > > I'm guessing something with dup() or dup2(), possibly? > > This should already work, unless the O_CLOEXEC flag has been set on the > file descriptor for stderr. The kernel should duplicate open > descriptors on fork(2), and exec(2) should maintain its association to > any already-open descriptors. Hmm then I don't understand the SIGPIPE. > But, which process was the one writing to the terminal? Do we know that > the destination of stderr was a PTY instead of being filtered through > some kind of kdeinit magic? Yes, no kdeinit involved. Here's the setup: $ bin/kcrashtest testAutoRestartDirectly starts test_crasher (with QProcess, using waitForFinished()) which crashes :) KCrash catches that, and since the Autorestart flag is set, sets the env var KCRASH_AUTO_RESTARTED, and starts test_crasher again (it checks that env var, so no infinite loop) The first test_crasher then exits (after waitpid(), but that's just waiting for the child to start, right?). That second test_crasher has debug output which leads to the SIGPIPE write(2, "11:16:24.313 test_crasher(22916)"..., 119) = -1 EPIPE (Broken pipe) Is this because QProcess set up something for catching stderr, which no longer exists when the process exits? Oh. proc.setProcessChannelMode(QProcess::ForwardedChannels); fixes it! Thanks for the rubber-ducking! Explaining the problem made me find the solution :-) (BTW both test_crashers also append one line to a logfile, which the unittest polls, that's how it checks that everything went fine). > All this makes me wonder if it wouldn't be possible (and preferable) to > have kdeinit handle the restart if it was responsible for the initial > launch. There might not be a kdeinit running at all, when a random Qt app links to KCrash and there is no plasma workspace around. Like on the CI, too. I want a modular solution :) -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: kcrash, fork, and stdout/stderr
On Sun, Dec 03, 2017 at 11:54:21AM +0100, David Faure wrote: > I'm trying to fix a bug (caught by CI) where KCrash, with the AutoRestart > flag, > restarts the crashing process directly (fork+execve) rather than via kdeinit. > > The first process then exits, and whenever the child writes to stderr, it > gets > a SIGPIPE. Oops. > > How do I set things up so that the child's stderr is basically what the > parent's stderr was, in a way that works even if the parent exits? > > I'm guessing something with dup() or dup2(), possibly? This should already work, unless the O_CLOEXEC flag has been set on the file descriptor for stderr. The kernel should duplicate open descriptors on fork(2), and exec(2) should maintain its association to any already-open descriptors. In that sense, calls like dup(2) are more often used to try to reconnect open descriptors onto more appropriate file descriptors (e.g. to have stdout and stderr refer to the same file descriptor) than to act as a 'refcount' for the underlying descriptor. But, which process was the one writing to the terminal? Do we know that the destination of stderr was a PTY instead of being filtered through some kind of kdeinit magic? If there's a filter pipeline involved in stderr processing then the whole chain would need to still be present after fork/exec, not just the child. All this makes me wonder if it wouldn't be possible (and preferable) to have kdeinit handle the restart if it was responsible for the initial launch. Regards, - Michael Pyne
kcrash, fork, and stdout/stderr
I'm trying to fix a bug (caught by CI) where KCrash, with the AutoRestart flag, restarts the crashing process directly (fork+execve) rather than via kdeinit. The first process then exits, and whenever the child writes to stderr, it gets a SIGPIPE. Oops. How do I set things up so that the child's stderr is basically what the parent's stderr was, in a way that works even if the parent exits? I'm guessing something with dup() or dup2(), possibly? -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
