Date: Tue, 03 Sep 2019 14:06:02 -0000 From: Bruce Evans <brde@optusnet.com.au> To: Konstantin Belousov <kostikbel@gmail.com> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r345696 - head/lib/libvgl Message-ID: <20190330142319.I1011@besplex.bde.org> In-Reply-To: <20190329182100.GZ1923@kib.kiev.ua> References: <201903291557.x2TFv9AW097226@repo.freebsd.org> <20190329182100.GZ1923@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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. > >> + } >> } >> >> static void > Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190330142319.I1011>