From patchwork Tue Jan 28 13:03:14 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 22658 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id C2B25C323E for ; Tue, 28 Jan 2025 13:03:20 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5B03368560; Tue, 28 Jan 2025 14:03:20 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=protonmail.com header.i=@protonmail.com header.b="nUnxPQvM"; dkim-atps=neutral Received: from mail-40133.protonmail.ch (mail-40133.protonmail.ch [185.70.40.133]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7220768557 for ; Tue, 28 Jan 2025 14:03:17 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1738069396; x=1738328596; bh=8le9BDiluVhvE6bS3IDOvjbE9lOeI2hp6AyJOkHeS5M=; h=Date:To:From:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=nUnxPQvMKfrbWL87/Lso9Pk0OhWy3TrSdL8nxlqkYyQVEHCtwa8L6wx/Vj1hrp4KG 9z1TelSn814ZJA6LJqctzhJA1dhtz1jBPvy7fviTtfKW1HSkzdYhoMfTilLuS+Q8EY lXMMmcBuXIblQSpQPkvjppT8IRzaal5zEFjrXzXEXf7WW4mtRNqb1YI0SpERiT7hT7 UBbf5cU7JnkoTUvpHQbzYAEoD2QB19hhJvpJJbEegUSay9kT0iNZJRclI4ltaUYSk6 B8VyEX24WM3SGC+yIZ3vIthd73wIIZPzd/1aGLLmh3QJSxNcRLBi2Jf0ICuIZL699C UiATNaXpFqFJw== Date: Tue, 28 Jan 2025 13:03:14 +0000 To: libcamera-devel@lists.libcamera.org From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Subject: [RFC PATCH v1] libcamera: thread: Fix event dispatcher creation Message-ID: <20250128130308.509297-2-pobrn@protonmail.com> In-Reply-To: <20250128130308.509297-1-pobrn@protonmail.com> References: <20250128130308.509297-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: c8d93ac1df3e9d1c6871197173378f27b7fce573 MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Use an appropriate compare-and-exchange atomic instruction to set the event dispatcher pointer. This ensures that there is only ever a single event dispatcher associated with a given thread. Consider e.g. the following series of events: 1. thread 0 :: if (!data_->dispatcher_.load(std::memory_order_relaxed)) // reads nullptr and proceeds into the `if`'s block 2. thread 1 :: if (!data_->dispatcher_.load(std::memory_order_relaxed)) // reads nullptr and proceeds into the `if`'s block 3. thread 0 :: data_->dispatcher_.store(new EventDispatcherPoll(), std::memory_order_release); // creates dispatcher and sets pointer 4. thread 1 :: data_->dispatcher_.store(new EventDispatcherPoll(), std::memory_order_release); // creates dispatcher and sets pointer At the end, one of the event dispatchers is leaked, and furthermore the `eventDispatcher()` calls might return different values in the calling threads. Signed-off-by: Barnabás Pőcze --- src/libcamera/base/thread.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) -- 2.48.1 diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp index de60567f6..93ec7b5a0 100644 --- a/src/libcamera/base/thread.cpp +++ b/src/libcamera/base/thread.cpp @@ -518,11 +518,16 @@ pid_t Thread::currentId() */ EventDispatcher *Thread::eventDispatcher() { - if (!data_->dispatcher_.load(std::memory_order_relaxed)) - data_->dispatcher_.store(new EventDispatcherPoll(), - std::memory_order_release); + if (EventDispatcher *dispatcher = data_->dispatcher_.load(std::memory_order_acquire)) + return dispatcher; - return data_->dispatcher_.load(std::memory_order_relaxed); + auto dispatcher = std::make_unique(); + EventDispatcher *expected = nullptr; + + if (data_->dispatcher_.compare_exchange_strong(expected, dispatcher.get(), std::memory_order_acq_rel)) + return dispatcher.release(); + + return expected; } /**