From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: glibc mutex deadlock in signal handler Date: Fri, 04 Sep 2015 15:40:58 +0200 Message-ID: References: <87y4gn5ijr.fsf@igel.home> <20150904092355.GA524@sigill.intra.peff.net> <20150904130448.GB25501@sigill.intra.peff.net> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Cc: Andreas Schwab , Junio C Hamano , git@vger.kernel.org To: Jeff King X-From: git-owner@vger.kernel.org Fri Sep 04 15:41:10 2015 Return-path: Envelope-to: gcvg-git-2@plane.gmane.org Received: from vger.kernel.org ([209.132.180.67]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1ZXrEn-0000Im-Hy for gcvg-git-2@plane.gmane.org; Fri, 04 Sep 2015 15:41:05 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933168AbbIDNlC (ORCPT ); Fri, 4 Sep 2015 09:41:02 -0400 Received: from mx2.suse.de ([195.135.220.15]:40179 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932908AbbIDNlA (ORCPT ); Fri, 4 Sep 2015 09:41:00 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 0A21CAD44; Fri, 4 Sep 2015 13:40:59 +0000 (UTC) In-Reply-To: <20150904130448.GB25501@sigill.intra.peff.net> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.5 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: On Fri, 04 Sep 2015 15:04:48 +0200, Jeff King wrote: > > On Fri, Sep 04, 2015 at 11:35:57AM +0200, Takashi Iwai wrote: > > > > Hmm, is there is any reason to just pass an "in_signal" flag to > > > wait_for_pager(), to avoid duplicating the logic? > > > > Just because wait_for_pager() itself is an atexit hook that can't take > > an argument, so we'd need to split to a new function. I don't mind > > either way. The revised patch is below. > > Ah, right. That's unfortunate, but I think I prefer adding the extra > wrapper to duplicating the contents of the function. > > > -- 8< -- > > From: Takashi Iwai > > Subject: [PATCH v2] pager: don't use unsafe functions in signal handlers > > [...] > > This looks good to me. Do you plan on fixing any of the other handlers > (you don't have to; I just want to know if somebody is planning to work > on it). Heh, I'd like to leave the rest for professionals :) > The pattern of atexit and signal handlers is repeated in several places, > and it seems like we will have to add the same in_signal boilerplate in > each instance. I wonder if we should provide a global "register_cleanup" > that takes a "void (*func)(int in_signal))" function pointer, and: > > 1. Adds it to a list (ideally in a way that is atomic if we get > interrupted while adding to the list). > > 2. If not already run, registers an atexit() handler and > sigchain_push_common for a meta-handler which runs through the list > and runs each handler. > > It's not a _huge_ amount of boilerplate code we'd be saving, but at > least conforming to the "in_signal" function template would make people > think twice about what they're doing inside the cleanup function. :) Or, we may have a global in_signal flag. Of course, this is bad if git itself is multi-threaded, though. thanks, Takashi