From owner-freebsd-stable@freebsd.org Thu Jun 15 03:38:06 2017 Return-Path: Delivered-To: freebsd-stable@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 27FB1BFF417 for ; Thu, 15 Jun 2017 03:38:06 +0000 (UTC) (envelope-from eugen@grosbein.net) Received: from hz.grosbein.net (hz.grosbein.net [78.47.246.247]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "hz.grosbein.net", Issuer "hz.grosbein.net" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 9DC7B109C; Thu, 15 Jun 2017 03:38:05 +0000 (UTC) (envelope-from eugen@grosbein.net) Received: from eg.sd.rdtc.ru (root@eg.sd.rdtc.ru [62.231.161.221]) by hz.grosbein.net (8.15.2/8.15.2) with ESMTPS id v5F3bxvW095814 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 15 Jun 2017 05:38:00 +0200 (CEST) (envelope-from eugen@grosbein.net) X-Envelope-From: eugen@grosbein.net X-Envelope-To: kostikbel@gmail.com Received: from [10.58.0.4] ([10.58.0.4]) by eg.sd.rdtc.ru (8.15.2/8.15.2) with ESMTPS id v5F3bt1W058015 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Thu, 15 Jun 2017 10:37:55 +0700 (+07) (envelope-from eugen@grosbein.net) Subject: Re: syslog() thread unsafety To: Konstantin Belousov References: <59413EF0.20608@grosbein.net> <20170614141238.GM2088@kib.kiev.ua> <594166CB.60709@grosbein.net> <20170614170904.GR2088@kib.kiev.ua> Cc: FreeBSD Stable , Gleb Smirnoff From: Eugene Grosbein Message-ID: <5942010E.40907@grosbein.net> Date: Thu, 15 Jun 2017 10:37:50 +0700 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <20170614170904.GR2088@kib.kiev.ua> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=0.3 required=5.0 tests=BAYES_00,LOCAL_FROM autolearn=no autolearn_force=no version=3.4.1 X-Spam-Report: * -2.3 BAYES_00 BODY: Bayes spam probability is 0 to 1% * [score: 0.0000] * 2.6 LOCAL_FROM From my domains X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on hz.grosbein.net X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 15 Jun 2017 03:38:06 -0000 15.06.2017 0:09, Konstantin Belousov wrote: > On Wed, Jun 14, 2017 at 11:39:39PM +0700, Eugene Grosbein wrote: >> 14.06.2017 21:12, Konstantin Belousov wrote: >> >>> If the issue is that mpd5 cancels logging thread, and this leaves the >>> mutex in the locked state, the right solution is to establish a cleanup >>> handler around the locked region. Note that this can only work if the >>> cancellation is in deferred mode, async mode is unsafe by definition. >>> >>> Try something like this, untested even a minimal bit. >> >> [skip] >> >> I've given it a spin with unpatched mpd5 and it seems to work just fine now. >> I'm curious, should these two lines be swapped? >> >> + THREAD_LOCK(); >> + pthread_cleanup_push(syslog_cancel_cleanup, NULL); >> >> It seems it could be a race between another thread's pthread_cancel() >> and pthread_cleanup_push() here. > As I mentioned in my previous reply, you can only rely on some > guarantees for the deferred cancellation mode. In this mode, only some > calls are allowed to become cancellation points. In particular, the > cancellation cannot happen between THREAD_LOCK() and push(). Forgot to mention that mpd5 does not change the mode to PTHREAD_CANCEL_ASYNCHRONOUS from the default deferred. > That is, popen() looks safe. >> getlogin.c >> lib/libc/stdio: findfp.c, fclose.c > I already wrote about fclose() above, might be it indeed should grow the > cleanup handler for the general case. I do not see any code which would > need cancellation protection in findfp, what exactly do you mean there ? > Same for getlogin(). That was just quick scan for THREAD_LOCK() :-) I'm new to these details of pthreads. >> Please consider committing the fix at least for syslog.c > Confirm that it fixed your case, then I will commit the change. Well, my stress test for mpd5 has been running for several hours and no hang still. Without your patch, it locked in a minute, so it seems the case is fixed, thanks! Eugene Grosbein