svn commit: r345696 - head/lib/libvgl
Konstantin Belousov
kostikbel at gmail.com
Sat Mar 30 09:46:12 UTC 2019
On Sat, Mar 30, 2019 at 03:24:40PM +1100, Bruce Evans wrote:
> On Fri, 29 Mar 2019, Konstantin Belousov wrote:
>
> > On Fri, Mar 29, 2019 at 03:57:09PM +0000, Bruce Evans wrote:
> >> Author: bde
> >> Date: Fri Mar 29 15:57:08 2019
> >> New Revision: 345696
> >> URL: https://svnweb.freebsd.org/changeset/base/345696
> >>
> >> Log:
> >> Fix endless loops for handling SIGBUS and SIGSEGV.
> >>
> >> r80270 has the usual wrong fix for unsafe signal handling -- just set
> >> a flag and return to let an event loop check the flag and do safe
> >> handling. This never works for signals like SIGBUS and SIGSEGV that
> >> repeat and works poorly for others unless the application has an event
> >> loop designed to support this.
> >>
> >> For these signals, clean up unsafely as before, except for arranging that
> >> nested signals are fatal and forcing a nested signal if the cleanup doesn't
> >> cause one.
> >>
> >> Modified:
> >> head/lib/libvgl/main.c
> >>
> >> Modified: head/lib/libvgl/main.c
> >> ==============================================================================
> >> --- head/lib/libvgl/main.c Fri Mar 29 15:20:48 2019 (r345695)
> >> +++ head/lib/libvgl/main.c Fri Mar 29 15:57:08 2019 (r345696)
> >> ...
> >> @@ -107,14 +107,22 @@ struct vt_mode smode;
> >> }
> >>
> >> static void
> >> -VGLAbort(int arg __unused)
> >> +VGLAbort(int arg)
> >> {
> >> + sigset_t mask;
> >> +
> >> VGLAbortPending = 1;
> >> signal(SIGINT, SIG_IGN);
> >> signal(SIGTERM, SIG_IGN);
> >> - signal(SIGSEGV, SIG_IGN);
> >> - signal(SIGBUS, SIG_IGN);
> >> signal(SIGUSR2, SIG_IGN);
> >> + if (arg == SIGBUS || arg == SIGSEGV) {
> >> + signal(arg, SIG_DFL);
> >> + sigemptyset(&mask);
> >> + sigaddset(&mask, arg);
> >> + sigprocmask(SIG_UNBLOCK, &mask, NULL);
> >> + VGLEnd();
> >> + kill(getpid(), arg);
> > This of course misses the siginfo information from the real fault.
>
> It is in the nested signal frame.
>
> > Why SIGBUS/SIGSEGV are caught at all ?
>
> Otherwise, the screen is left in an application mode that the kernel
> doesn't support (except to not write to the screen, so that the
> application has full control).
>
> Of course, it is bad for libraries to catch signals. The man page warns
> about installing signal handlers and trying to call VGLEnd() and exit(3)
> [from the signal handler] to end the program, although it is important
> to use VGLEnd() to end the program. I haven't tried installing signal
> handlers in programs, but my main application is another graphics library
> that wraps libvgl, and it installs an atexit() handler which calls its
> cleanup function which calls VGLEnd(). libvgl should do the same. The
> cleanup is safe in exit() provided calling exit() is safe.
>
> The man page has general nonsense recommending setting a flag and not
> terminating the program in signal handlers. This was added in the same
> commit that completely broke the SIGBUS and SIGSEGV handling using this
> method. A much longer lesson is needed to describe how to use this
> method correctly. SA_RESTART must be turned off for all signals, and
> this gives the complexity of handling EINTR returns from all syscalls
> affected by SA_RESTART ...
>
> The man page has almost enough details about which signals the library
> catches. It doesn't mention SIGUSR1 (used for screen switches) or
> SIGUSR2 (used for mouse signals). I plan to add use of SIGIO to fix
> busy-waiting for keyboard events. select() might work for the keyboard,
> but I doubt that it works for the mouse.
>
> What should happen is if the application installs signal handlers and
> does unsafe things not including calling VGLEnd(), then nested SIGBUS's
> and SIGSEGV's should reach the above cleanup. Applications should not
> catch SIGBUS or SIGSEGV unless they can clean up better than the above.
> It is possible to clean up better than the above, but not in applications.
> The above can check a soft signal mask flag like the one set by INTOFF()
> and not do so much when it is set. When it is not set, I think restoring
> the screen mode is safe in signal handlers, and only things like free()
> and printf() are not safe in signal handlers.
I am more about not doing kill(2) from the handler, instead do plain
return to the context which caused the original signal.
More information about the svn-src-all
mailing list