From owner-svn-src-all@freebsd.org Sat Mar 30 09:46:12 2019 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 2BA1E156C989; Sat, 30 Mar 2019 09:46:12 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 47A3C77BAF; Sat, 30 Mar 2019 09:46:11 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id x2U9k1Mf079595 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Sat, 30 Mar 2019 11:46:04 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua x2U9k1Mf079595 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id x2U9jwdT079594; Sat, 30 Mar 2019 11:45:58 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Sat, 30 Mar 2019 11:45:58 +0200 From: Konstantin Belousov To: Bruce Evans 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: <20190330094558.GA1923@kib.kiev.ua> References: <201903291557.x2TFv9AW097226@repo.freebsd.org> <20190329182100.GZ1923@kib.kiev.ua> <20190330142319.I1011@besplex.bde.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190330142319.I1011@besplex.bde.org> User-Agent: Mutt/1.11.4 (2019-03-13) X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on tom.home X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 Mar 2019 09:46:12 -0000 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.