From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on gnuweeb.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=ALL_TRUSTED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NO_DNS_FOR_FROM,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.6 Received: from gnuweeb.org (unknown [51.81.211.47]) by gnuweeb.org (Postfix) with ESMTPSA id D0A22832D6; Sat, 11 Mar 2023 11:28:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gnuweeb.org; s=default; t=1678534101; bh=/Pdi+tK1AhN7Q5NDGcJlkbQl1JJUBhCX/rik0pb4Bhs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=kUh0DuTkT2oDoTSVsadN90n9i76En18+ZxPfYgnGYpaBym7iZRZxpxJpUdCoJ9mNm 1bV35AllFIzu3IunJte0HP5IEe7t+fBQ22bFOiL4X8nYBra3kaywGRgdTa1u9RSnm1 95iJPCULbqy8QqsvGrDb1yxeTGbO/O/MfJt387dzL4TiEz41Y4IXRas7P0va16n2fZ hM7Kn8OPXm//qgxyjLbxKZlGy6yN1hv/oDDO/ZxEgZ8pWTI/jFxrrj4tKhLzEue++M RCqI7cdRKcV5eA7liFVFUjnNVU5Mn0ekyXCWBpr5jTfj6suWhfl1RYQ1WVd2pJXrpE llZwSf0oS35aw== From: Alviro Iskandar Setiawan To: Ammar Faizi Cc: Alviro Iskandar Setiawan , Irvan Malik Azantha , GNU/Weeb Mailing List Subject: [RFC PATCH v1 2/3] core/thread: Fix undefined behavior in the C++ mutex implementation Date: Sat, 11 Mar 2023 11:28:09 +0000 Message-Id: <20230311112810.3670483-3-alviro.iskandar@gnuweeb.org> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20230311112810.3670483-1-alviro.iskandar@gnuweeb.org> References: <20230311112810.3670483-1-alviro.iskandar@gnuweeb.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: The current C++ mutex implementation is undefined behavior, specifically in the cond_wait() function because std::unique_lock is constructed with std::defer_lock while the same thread has acquired the lock. Also, right after that defer_lock, std::condition_variable calls wait() with a unique lock, not in a locked state. In such a situation, the correct construction is using std::adopt_lock. However, using std::adopt_lock leads to another issue. The issue is the lock will be released upon return in the cond_wait(). To solve the problem, introduce a new helper function, __cond_wait() which will release the lock with std::adopt_lock and then call it from cond_wait(). The cond_wait() then acquires the lock again before it returns. The result is that we correctly fulfill the __must_hold() semantic while conforming to the C++ mutex implementation. Signed-off-by: Alviro Iskandar Setiawan --- core/thread.cc | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/core/thread.cc b/core/thread.cc index 2ff81e5..54e06e7 100644 --- a/core/thread.cc +++ b/core/thread.cc @@ -181,13 +181,22 @@ int cond_init(struct cond_struct **c) return 0; } -int cond_wait(struct cond_struct **c, struct mutex_struct **m) +static int __cond_wait(struct cond_struct *c, struct mutex_struct *m) { - std::unique_lock lock((*m)->mutex, std::defer_lock); - (*c)->cond.wait(lock); + std::unique_lock lock(m->mutex, std::adopt_lock); + c->cond.wait(lock); return 0; } +int cond_wait(struct cond_struct **c, struct mutex_struct **m) +{ + int ret; + + ret = __cond_wait(*c, *m); + mutex_lock(m); + return ret; +} + int cond_signal(struct cond_struct **c) { (*c)->cond.notify_one(); -- Alviro Iskandar Setiawan