From patchwork Fri Jan 9 14:40:50 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 427101 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 365F5140188 for ; Sat, 10 Jan 2015 01:41:04 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; q=dns; s= default; b=uMKGoXK//RhJz4vF3gAMsLvnqkKEZnRadUaUQwo+sHea+uWHimRqA BlqHOlVtAVMszLsHwY39eClXls6VzJrJyF4sCqBpImXPJtDi3vJWPg66mgsFN2P5 5l155x0OJJDgfoiOOER69vYvrp8q4g51uqBosLtBmhRNN/RhN3ynss= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; s= default; bh=hR4ClWfSxgSAVvAhhpNUufEMFvY=; b=Xl3aAeF6xbeTHYKsDzkk nhdjtcgz0vTdu8ivjlgtpYpTLOOZmN/JweBgxH/bQnTdQZ01K1JpNMQmqWhzPw+W owWwTLMLjpSlvDDTZfRlTyYrWdRlmL8XQFh8YOQEZdN4HFgHCNig3AtNsoWXZGtL AKOnNFh8du1p4QgVs1QwxsU= Received: (qmail 21978 invoked by alias); 9 Jan 2015 14:40:56 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 21961 invoked by uid 89); 9 Jan 2015 14:40:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 09 Jan 2015 14:40:53 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t09EeqTL003575 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Fri, 9 Jan 2015 09:40:52 -0500 Received: from localhost (ovpn-116-142.ams2.redhat.com [10.36.116.142]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t09Eepnd007694; Fri, 9 Jan 2015 09:40:51 -0500 Date: Fri, 9 Jan 2015 14:40:50 +0000 From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [patch] libstdc++/60966 fix packaged_task also Message-ID: <20150109144050.GR3360@redhat.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) The race conditions fixed in PR 60966 can also happen when using std::packaged_task (on the release branches only, the fix applied to the trunk covers all cases). The problem is that the mutex protecting the result in the shared state is unlocked before the _M_cond.notify_all() call. This leaves a small window where threads waiting on the future can see the result (and so return from a waiting function) before the notify_all(). If the waiting thread destroys the future *and* the packaged_task as soon as the waiting function returns, then _M_cond will be destroyed before the notify_all() call on it, resulting in undefined behaviour (typically this means blocking forever in the pthread_cond_broadcast() call). This patch uses the same approach as done for std::promise on the release branches: increasing the ref-count on the shared state until the function setting the result has completed. This ensures that the shared state (and its condition_variable member) will not be destroyed until after the _M_cond.notify_all() call. Thanks to Barry Revzin for finding the problem and Michael Karcher for debugging it. The original fixes for 60966 solved the problem for std::promise, this solves it for std::packaged_task. The only other types of asynchronous result providers are those created by std::async, but for those types the waiting functions already block in _M_complete_async() so cannot return before the notify_all() call happens. Tested x86_64-linux, committed to the 4.9 and 4.8 branches. (No new testcase, because the hang only shows up occasionally after thousands of iterations, or when using valgrind.) commit 053f27a4ffbb7cadd780c0b28507aaff98a38824 Author: Jonathan Wakely Date: Fri Jan 9 13:35:42 2015 +0000 PR libstdc++/60966 * include/std/future (packaged_task::operator()): Increment the reference count on the shared state until the function returns. diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future index d446b9d..6523cea 100644 --- a/libstdc++-v3/include/std/future +++ b/libstdc++-v3/include/std/future @@ -1450,7 +1450,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION operator()(_ArgTypes... __args) { __future_base::_State_base::_S_check(_M_state); - _M_state->_M_run(std::forward<_ArgTypes>(__args)...); + auto __state = _M_state; + __state->_M_run(std::forward<_ArgTypes>(__args)...); } void