From patchwork Thu Feb 2 20:56:57 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Lerda X-Patchwork-Id: 89553 Received: from [127.0.0.1] by www.linuxtv.org with esmtp (Exim 4.92) (envelope-from ) id 1pNge5-00EM4k-VW; Thu, 02 Feb 2023 20:57:25 +0000 Received: from zkko.ml ([78.198.177.192] helo=mail.zkko.ml) by www.linuxtv.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pNge3-00EM4M-00 for vdr@linuxtv.org; Thu, 02 Feb 2023 20:57:24 +0000 Received: from over.fork.zz (over.fork.zz [192.168.0.155]) by mail.zkko.ml (8.16.1/8.16.1) with ESMTPS id 312KvJN0031555 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=FAIL); Thu, 2 Feb 2023 21:57:20 +0100 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=zkko.ml; s=mail; t=1675371440; bh=wcVzgULcCkAp4iQ8/njJIlMWZ+I=; h=From:To:Cc:Subject:Date:From; b=kPEbRnLQpJs1gapblrvSY2MlrQ4JV18wQssO8iWJii435k3/XmdqK2TslO5ekRdSd vXuwJUooNZQxX+w00MWJGjzxUQfRlIzdmw/+5k1KLMihTPbjXaeb2YxbxnHngqe5Yr wXHcLqSY2zCdC6MVSnY0eIDe0AYljhX32HWFPCfw= Received: from over.fork.zz (localhost.fork.zz [127.0.0.1]) by over.fork.zz (8.16.1/8.16.1) with ESMTPS id 312KvJ1X021930 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Thu, 2 Feb 2023 21:57:19 +0100 Received: (from patrick@localhost) by over.fork.zz (8.16.1/8.16.1/Submit) id 312KvI4L021929; Thu, 2 Feb 2023 21:57:18 +0100 X-Authentication-Warning: over.fork.zz: patrick set sender to patrick9876@free.fr using -f From: Patrick Lerda To: vdr@linuxtv.org Date: Thu, 2 Feb 2023 21:56:57 +0100 Message-Id: X-Mailer: git-send-email 2.39.1 MIME-Version: 1.0 X-LSpam-Score: -1.6 (-) X-LSpam-Report: No, score=-1.6 required=5.0 tests=BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, FREEMAIL_ENVFROM_END_DIGIT=0.25, FREEMAIL_FROM=0.001 autolearn=no autolearn_force=no Subject: [vdr] [PATCH] Fix cThread related race conditions X-BeenThere: vdr@linuxtv.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: VDR Mailing List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: VDR Mailing List Errors-To: vdr-bounces@linuxtv.org Sender: "vdr" This change fixes the following issue: ==15457==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000002d0 (pc 0x7fd2f4301710 bp 0x7fd2b5552a30 sp 0x7fd2b5552988 T228) ==15457==The signal is caused by a READ memory access. ==15457==Hint: address points to the zero page. #0 0x7fd2f4301710 in pthread_detach (/lib64/libc.so.6+0x8b710) #1 0xdb4430 in cThread::Start() vdr-2.6.3/thread.c:317 #2 0x7fd2e5ea8fc4 in cIptvStreamer::Open() vdr-2.6.3/PLUGINS/src/iptv/streamer.c:66 #3 0x7fd2e5e1e8c2 in cIptvDevice::OpenDvr() vdr-2.6.3/PLUGINS/src/iptv/device.c:342 #4 0x634903 in cDevice::Action() vdr-2.6.3/device.c:1714 #5 0xdb644a in cThread::StartThread(cThread*) vdr-2.6.3/thread.c:293 #6 0x7fd2f4300729 (/lib64/libc.so.6+0x8a729) #7 0x7fd2f437d0bb (/lib64/libc.so.6+0x1070bb) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV (/lib64/libc.so.6+0x8b710) in pthread_detach Thread T228 (device 5 receiv) created by T222 (cLiveStreamer s) here: #0 0x7fd2f52d4716 in pthread_create (/usr/lib64/libasan.so.6+0x59716) #1 0xdb437b in cThread::Start() vdr-2.6.3/thread.c:316 #2 0x63a3ad in cDevice::AttachReceiver(cReceiver*) vdr-2.6.3/device.c:1844 #3 0x7fd2e8162c56 in cDummyReceiver::Create(cDevice*) vdr-2.6.3/PLUGINS/src/vnsiserver/videoinput.c:472 #4 0x7fd2e816dbc2 in cVideoInput::Open(cChannel const*, int, cVideoBuffer*) vdr-2.6.3/PLUGINS/src/vnsiserver/videoinput.c:530 #5 0x7fd2e80d78d3 in cLiveStreamer::Action() vdr-2.6.3/PLUGINS/src/vnsiserver/streamer.c:318 #6 0xdb644a in cThread::StartThread(cThread*) vdr-2.6.3/thread.c:293 #7 0x7fd2f4300729 (/lib64/libc.so.6+0x8a729) --- thread.c | 25 +++++++++++++++---------- thread.h | 11 +++++++---- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/thread.c b/thread.c index 93eb8c0..21be7a4 100644 --- a/thread.c +++ b/thread.c @@ -312,13 +312,16 @@ bool cThread::Start(void) cCondWait::SleepMs(THREAD_STOP_SLEEP); } if (!active) { - active = running = true; - if (pthread_create(&childTid, NULL, (void *(*) (void *))&StartThread, (void *)this) == 0) { - pthread_detach(childTid); // auto-reap + pthread_t localTid; + running = true; + if (pthread_create(&localTid, NULL, (void *(*) (void *))&StartThread, (void *)this) == 0) { + pthread_detach(localTid); // auto-reap + childTid = localTid; + active = true; } else { LOG_ERROR; - active = running = false; + running = false; return false; } } @@ -339,11 +342,12 @@ bool cThread::Active(void) // performed but no signal is actually sent. // int err; - if ((err = pthread_kill(childTid, 0)) != 0) { + const pthread_t localTid = childTid; + if ((err = pthread_kill(localTid, 0)) != 0) { if (err != ESRCH) LOG_ERROR; - childTid = 0; - active = running = false; + if (active && childTid == localTid) + active = running = false; } else return true; @@ -355,6 +359,7 @@ void cThread::Cancel(int WaitSeconds) { running = false; if (active && WaitSeconds > -1) { + const pthread_t localTid = childTid; if (WaitSeconds > 0) { for (time_t t0 = time(NULL) + WaitSeconds; time(NULL) < t0; ) { if (!Active()) @@ -363,9 +368,9 @@ void cThread::Cancel(int WaitSeconds) } esyslog("ERROR: %s thread %d won't end (waited %d seconds) - canceling it...", description ? description : "", childThreadId, WaitSeconds); } - pthread_cancel(childTid); - childTid = 0; - active = false; + pthread_cancel(localTid); + if (active && childTid == localTid) + active = false; } } diff --git a/thread.h b/thread.h index 16c4bd7..06046ea 100644 --- a/thread.h +++ b/thread.h @@ -13,6 +13,7 @@ #include #include #include +#include typedef pid_t tThreadId; @@ -56,7 +57,7 @@ class cRwLock { private: pthread_rwlock_t rwlock; int locked; - tThreadId writeLockThreadId; + std::atomic writeLockThreadId; public: cRwLock(bool PreferWriter = false); ~cRwLock(); @@ -79,9 +80,11 @@ public: class cThread { friend class cThreadLock; private: - bool active; - bool running; - pthread_t childTid; + std::atomic_bool active; + std::atomic_bool running; + std::atomic childTid; + ///< Assume that the content of childTid is valid when the class member + ///< active is set to true and undefined otherwise. tThreadId childThreadId; cMutex mutex; char *description;