{"id":12368,"url":"https://patchwork.libcamera.org/api/1.1/patches/12368/?format=json","web_url":"https://patchwork.libcamera.org/patch/12368/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20210523023342.13141-1-laurent.pinchart@ideasonboard.com>","date":"2021-05-23T02:33:42","name":"[libcamera-devel] android: camera_worker: Process all queued requests when stopping","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"21052e256cd25f72b71b16056fbae7389fa58293","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/1.1/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/12368/mbox/","series":[{"id":2050,"url":"https://patchwork.libcamera.org/api/1.1/series/2050/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2050","date":"2021-05-23T02:33:42","name":"[libcamera-devel] android: camera_worker: Process all queued requests when stopping","version":1,"mbox":"https://patchwork.libcamera.org/series/2050/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/12368/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/12368/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 510D5C3201\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 23 May 2021 02:33:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EA5C16891A;\n\tSun, 23 May 2021 04:33:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 16F3960510\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 23 May 2021 04:33:49 +0200 (CEST)","from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 84A982A8;\n\tSun, 23 May 2021 04:33:48 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ITlKT5ds\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621737228;\n\tbh=L5kTxSM6vCCkM0eLdem83a0TViHxARO0NvRH8ABhBHk=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=ITlKT5dsZ6ti0Um/mtTNlbBWGJhq+kPwbfSbixNoTDT8DksDMGw+auEoQxEm6yZ9h\n\t2zmc5QWB7m++bv0QgVgYCZaF0p0Giw4B/h2CVQcO7tZqJfSo/DDm3dc4DYJbiZpxG3\n\tBupngHMBaXS9tN8xzOPIRQ6QTBKWAvJ599o/suhg=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Sun, 23 May 2021 05:33:42 +0300","Message-Id":"<20210523023342.13141-1-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.28.1","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH] android: camera_worker: Process all\n\tqueued requests when stopping","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"When stopping the camera worker, queuedRequest() calls may have queued\nasynchronous function invocation messages to the worker thread, and some\nof those messages may not have been processed yet. The messages will\nstay in the thread's queue until the camera worker is restarted (when\nthe camera service will start a new capture session). At that point,\nthey will be dispatched, which will cause a crash due to the\nCaptureRequest passed to processRequest() having been deleted by\nCameraDevice::stop() calling descriptors_.clear().\n\nFix this by forcing dispatching of all function invocation messages when\nstopping the camera worker thread. Note that this is inherently racy, as\nmore queueRequest() calls may arrive from the camera service while we're\nstopping. This race condition will be addressed by a subsequent patch\nseries.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\nI haven't tested this patch, as I'm currently rebuilding a new Soraka\nimage. Hiro, could you give it a try to see if it fixes the problem\nyou've reported ?\n---\n src/android/camera_worker.cpp | 14 ++++++++++----\n src/android/camera_worker.h   |  6 ++++--\n 2 files changed, 14 insertions(+), 6 deletions(-)","diff":"diff --git a/src/android/camera_worker.cpp b/src/android/camera_worker.cpp\nindex 300ddde03645..9f727826e23f 100644\n--- a/src/android/camera_worker.cpp\n+++ b/src/android/camera_worker.cpp\n@@ -52,18 +52,24 @@ void CaptureRequest::queue()\n  */\n CameraWorker::CameraWorker()\n {\n-\tworker_.moveToThread(&thread_);\n+\tworker_.moveToThread(this);\n }\n \n void CameraWorker::start()\n {\n-\tthread_.start();\n+\tThread::start();\n }\n \n void CameraWorker::stop()\n {\n-\tthread_.exit();\n-\tthread_.wait();\n+\texit();\n+\twait();\n+}\n+\n+void CameraWorker::run()\n+{\n+\texec();\n+\tdispatchMessages(Message::Type::InvokeMessage);\n }\n \n void CameraWorker::queueRequest(CaptureRequest *request)\ndiff --git a/src/android/camera_worker.h b/src/android/camera_worker.h\nindex 64b1658b61b4..e289ef9b8655 100644\n--- a/src/android/camera_worker.h\n+++ b/src/android/camera_worker.h\n@@ -42,7 +42,7 @@ private:\n \tstd::unique_ptr<libcamera::Request> request_;\n };\n \n-class CameraWorker\n+class CameraWorker : private libcamera::Thread\n {\n public:\n \tCameraWorker();\n@@ -52,6 +52,9 @@ public:\n \n \tvoid queueRequest(CaptureRequest *request);\n \n+protected:\n+\tvoid run() override;\n+\n private:\n \tclass Worker : public libcamera::Object\n \t{\n@@ -63,7 +66,6 @@ private:\n \t};\n \n \tWorker worker_;\n-\tlibcamera::Thread thread_;\n };\n \n #endif /* __ANDROID_CAMERA_WORKER_H__ */\n","prefixes":["libcamera-devel"]}