[{"id":14599,"web_url":"https://patchwork.libcamera.org/comment/14599/","msgid":"<YAbT5XWeuqOLIxbQ@pendragon.ideasonboard.com>","date":"2021-01-19T12:43:17","subject":"Re: [libcamera-devel] [PATCH 1/2] cam: event_loop: Stop queuing\n\tcalls when the event loop are exiting","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Tue, Jan 19, 2021 at 01:31:09PM +0100, Niklas Söderlund wrote:\n> Terminating the event loop with EventLoop::exit() does not grantee that\n> it will terminate. If the event loops 'call later' queue can be kept\n> non-empty by continuously queuing new calls using EventLoop::callLater()\n> either from a different thread or from callbacks in the loop itself\n> EventLoop::dispatchCalls() will never complete and the loop will run\n> forever.\n> \n> Solve this by not accepting new callbacks once the event loop is\n> exiting.\n> \n> Reported-by: Sebastian Fricke <sebastian.fricke@posteo.net>\n> Fixes: f49e93338b6309a6 (\"cam: event_loop: Add deferred calls support\")\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  src/cam/event_loop.cpp | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp\n> index 94c5d1d362455f33..34a55da30c537ac7 100644\n> --- a/src/cam/event_loop.cpp\n> +++ b/src/cam/event_loop.cpp\n> @@ -62,7 +62,7 @@ void EventLoop::interrupt()\n>  \n>  void EventLoop::callLater(const std::function<void()> &func)\n>  {\n> -\t{\n> +\tif (!exit_.load(std::memory_order_acquire)) {\n>  \t\tstd::unique_lock<std::mutex> locker(lock_);\n>  \t\tcalls_.push_back(func);\n>  \t}\n\nThat's a bit of a layering violation. Would it make sense to fix this\ndifferently, by ensuring that EventLoop::dispatchCalls() will only\nhandle deferred calls that are on the list when the function is called ?\nThis would also ensure that callLater(), when called from the same\nthread, will guarantee that pending events get processed before the\ndelayed call is processed.\n\nI mean something along those lines (only compile-tested).\n\ndiff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp\nindex 94c5d1d36245..320ade525a3f 100644\n--- a/src/cam/event_loop.cpp\n+++ b/src/cam/event_loop.cpp\n@@ -74,7 +74,7 @@ void EventLoop::dispatchCalls()\n {\n \tstd::unique_lock<std::mutex> locker(lock_);\n\n-\tfor (auto iter = calls_.begin(); iter != calls_.end(); ) {\n+\tfor (auto iter = calls_.begin(), end = calls_.end(); iter != end; ) {\n \t\tstd::function<void()> call = std::move(*iter);\n\n \t\titer = calls_.erase(iter);","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 12BDAC0F1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Jan 2021 12:43:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9660F68144;\n\tTue, 19 Jan 2021 13:43:36 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6A43268140\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Jan 2021 13:43:35 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E09C9813;\n\tTue, 19 Jan 2021 13:43:34 +0100 (CET)"],"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=\"V1WFMUdu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611060215;\n\tbh=tyKokXAzUg4l2hLc4TOAN4yHR868b6oQkSihiWn+b/A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=V1WFMUduM8ZK8RhN03d8L5jUCydkjFCzAP0EBFyK85lFjxt98qjno2TyhBm0j/4XD\n\tQ/lBItDUvf+RVIcxVj3BIZLec4wR9GEHC7t3ZTcxFh1KzgUiAGba1oxF2V2RykFCPv\n\tgZKDhilSnzle4XVXinIyDoXEH6/Y/N5BOJxFyefY=","Date":"Tue, 19 Jan 2021 14:43:17 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<YAbT5XWeuqOLIxbQ@pendragon.ideasonboard.com>","References":"<20210119123110.2976971-1-niklas.soderlund@ragnatech.se>\n\t<20210119123110.2976971-2-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210119123110.2976971-2-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH 1/2] cam: event_loop: Stop queuing\n\tcalls when the event loop are exiting","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14602,"web_url":"https://patchwork.libcamera.org/comment/14602/","msgid":"<YAbhwdkOljA3SGSt@oden.dyn.berto.se>","date":"2021-01-19T13:42:25","subject":"Re: [libcamera-devel] [PATCH 1/2] cam: event_loop: Stop queuing\n\tcalls when the event loop are exiting","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2021-01-19 14:43:17 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Tue, Jan 19, 2021 at 01:31:09PM +0100, Niklas Söderlund wrote:\n> > Terminating the event loop with EventLoop::exit() does not grantee that\n> > it will terminate. If the event loops 'call later' queue can be kept\n> > non-empty by continuously queuing new calls using EventLoop::callLater()\n> > either from a different thread or from callbacks in the loop itself\n> > EventLoop::dispatchCalls() will never complete and the loop will run\n> > forever.\n> > \n> > Solve this by not accepting new callbacks once the event loop is\n> > exiting.\n> > \n> > Reported-by: Sebastian Fricke <sebastian.fricke@posteo.net>\n> > Fixes: f49e93338b6309a6 (\"cam: event_loop: Add deferred calls support\")\n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  src/cam/event_loop.cpp | 2 +-\n> >  1 file changed, 1 insertion(+), 1 deletion(-)\n> > \n> > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp\n> > index 94c5d1d362455f33..34a55da30c537ac7 100644\n> > --- a/src/cam/event_loop.cpp\n> > +++ b/src/cam/event_loop.cpp\n> > @@ -62,7 +62,7 @@ void EventLoop::interrupt()\n> >  \n> >  void EventLoop::callLater(const std::function<void()> &func)\n> >  {\n> > -\t{\n> > +\tif (!exit_.load(std::memory_order_acquire)) {\n> >  \t\tstd::unique_lock<std::mutex> locker(lock_);\n> >  \t\tcalls_.push_back(func);\n> >  \t}\n> \n> That's a bit of a layering violation. Would it make sense to fix this\n> differently, by ensuring that EventLoop::dispatchCalls() will only\n> handle deferred calls that are on the list when the function is called ?\n> This would also ensure that callLater(), when called from the same\n> thread, will guarantee that pending events get processed before the\n> delayed call is processed.\n> \n> I mean something along those lines (only compile-tested).\n> \n> diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp\n> index 94c5d1d36245..320ade525a3f 100644\n> --- a/src/cam/event_loop.cpp\n> +++ b/src/cam/event_loop.cpp\n> @@ -74,7 +74,7 @@ void EventLoop::dispatchCalls()\n>  {\n>  \tstd::unique_lock<std::mutex> locker(lock_);\n> \n> -\tfor (auto iter = calls_.begin(); iter != calls_.end(); ) {\n> +\tfor (auto iter = calls_.begin(), end = calls_.end(); iter != end; ) {\n>  \t\tstd::function<void()> call = std::move(*iter);\n> \n>  \t\titer = calls_.erase(iter);\n\nI tried something like this but thought it was nastier then I proposed \nin this patch. I do however agree that what is proposed in my patch is \nalso not the most cleanest of solutions. The change you propose here \ndoes not work as intended, it is still venerable to an ever increasing \ncall_ vector.\n\nIf we would like to go with this solution how about something like this?  \nIt would also reduce the lock_ contention as a side effect.\n\ndiff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp\nindex 94c5d1d362455f33..4bfcc503ac1f3b83 100644\n--- a/src/cam/event_loop.cpp\n+++ b/src/cam/event_loop.cpp\n@@ -72,15 +72,13 @@ void EventLoop::callLater(const std::function<void()> &func)\n \n void EventLoop::dispatchCalls()\n {\n-       std::unique_lock<std::mutex> locker(lock_);\n+       std::list<std::function<void()>> calls;\n \n-       for (auto iter = calls_.begin(); iter != calls_.end(); ) {\n-               std::function<void()> call = std::move(*iter);\n+       {\n+               std::unique_lock<std::mutex> locker(lock_);\n+               calls = std::move(calls_);\n+       }\n \n-               iter = calls_.erase(iter);\n-\n-               locker.unlock();\n+       for (std::function<void()> call : calls)\n                call();\n-               locker.lock();\n-       }\n }\n\n> \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 B7C1CBD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Jan 2021 13:42:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 46B4F68146;\n\tTue, 19 Jan 2021 14:42:29 +0100 (CET)","from mail-lj1-x22b.google.com (mail-lj1-x22b.google.com\n\t[IPv6:2a00:1450:4864:20::22b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 101EC68140\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Jan 2021 14:42:28 +0100 (CET)","by mail-lj1-x22b.google.com with SMTP id n8so13138211ljg.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Jan 2021 05:42:27 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\td8sm2012021ljg.120.2021.01.19.05.42.26\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 19 Jan 2021 05:42:26 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"lmc85891\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=ZUBHvok7viM8p/UAxAvO3SeEaa01fyWlcAx5pc21pCQ=;\n\tb=lmc85891PXxqDnlQKGdsWtiA/7VsKHHR7cL2zGDcFsoJmLKYcYwGYCN0NGzSf4MRlw\n\tvwcGqGMjATLaFpZ2bxs9CpYE6P007nBkPESHn558bS16UO9+rzkQBD3A/vP5bqoCKVTA\n\tusAUCkowzz3HAoA/4CY18fMa/sSgoldgOI8+dAcrOTUXG4shi95+pG8jH5RAI+hmxN+h\n\tBIzBZBjJ3RjIeisblcbGfx0OFYT7NbE+HLK1fWgMc6HNwo7jBWHoBUK6jLb638H+aoYa\n\tJ9xP4KCdzCA6zPcz171cJfJfP9q/a94ajrkt8SXHUQa+I+6GDc0Gky6lNbABNCw22fdM\n\tFabw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=ZUBHvok7viM8p/UAxAvO3SeEaa01fyWlcAx5pc21pCQ=;\n\tb=QQxFy5Oq2uTxvqegp2a8/ojWbp9R4+lw+VauthMp8209kQpsf8A6Hhe8DDp380FJsA\n\tmqa2PLCSNkCy4Gx+7Rin7vkcBBM1llhaXdvQ1IgR26voedLxfk7w5gTDxrjskNlCYDj0\n\tTOk44O+S8SX6sS2yV/MNo3fenhNkjxAZ3AaV0QJmzHVgibzL57XRkBgiSXTaE4soXDjP\n\t/620qOxBfAgSiDKW8CK/0aJjLrGW8spk9vHrJcX/+UMLZF508nY/VBcqQaXcAYL1xsoG\n\tPvLVxB4pAtzP/dRPQShBlVL6lx8Nu5E/KAiyjsNrRpUwZW/bxnFD/EnYUAbAbPzPtoiX\n\tPAOA==","X-Gm-Message-State":"AOAM531jmYtRntT4UMGssuOQt/gkSFzimPC5Mj39tM/i6wQGPaabppzr\n\tRxDvWzsEkDGEYHLH2taktvfB5A==","X-Google-Smtp-Source":"ABdhPJz+nUKzXqtYHV3k1OSCnvDjBxgL4FIJs5ai6tJ/JhzL5aK+5KI4+SidryaFHZOlHGiyF5Y8cA==","X-Received":"by 2002:a2e:5756:: with SMTP id\n\tr22mr1993864ljd.481.1611063747440; \n\tTue, 19 Jan 2021 05:42:27 -0800 (PST)","Date":"Tue, 19 Jan 2021 14:42:25 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<YAbhwdkOljA3SGSt@oden.dyn.berto.se>","References":"<20210119123110.2976971-1-niklas.soderlund@ragnatech.se>\n\t<20210119123110.2976971-2-niklas.soderlund@ragnatech.se>\n\t<YAbT5XWeuqOLIxbQ@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<YAbT5XWeuqOLIxbQ@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/2] cam: event_loop: Stop queuing\n\tcalls when the event loop are exiting","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14603,"web_url":"https://patchwork.libcamera.org/comment/14603/","msgid":"<YAbnlFx/FoS0BkgC@oden.dyn.berto.se>","date":"2021-01-19T14:07:16","subject":"Re: [libcamera-devel] [PATCH 1/2] cam: event_loop: Stop queuing\n\tcalls when the event loop are exiting","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"On 2021-01-19 14:42:27 +0100, Niklas Söderlund wrote:\n> Hi Laurent,\n> \n> Thanks for your feedback.\n> \n> On 2021-01-19 14:43:17 +0200, Laurent Pinchart wrote:\n> > Hi Niklas,\n> > \n> > Thank you for the patch.\n> > \n> > On Tue, Jan 19, 2021 at 01:31:09PM +0100, Niklas Söderlund wrote:\n> > > Terminating the event loop with EventLoop::exit() does not grantee that\n> > > it will terminate. If the event loops 'call later' queue can be kept\n> > > non-empty by continuously queuing new calls using EventLoop::callLater()\n> > > either from a different thread or from callbacks in the loop itself\n> > > EventLoop::dispatchCalls() will never complete and the loop will run\n> > > forever.\n> > > \n> > > Solve this by not accepting new callbacks once the event loop is\n> > > exiting.\n> > > \n> > > Reported-by: Sebastian Fricke <sebastian.fricke@posteo.net>\n> > > Fixes: f49e93338b6309a6 (\"cam: event_loop: Add deferred calls support\")\n> > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > ---\n> > >  src/cam/event_loop.cpp | 2 +-\n> > >  1 file changed, 1 insertion(+), 1 deletion(-)\n> > > \n> > > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp\n> > > index 94c5d1d362455f33..34a55da30c537ac7 100644\n> > > --- a/src/cam/event_loop.cpp\n> > > +++ b/src/cam/event_loop.cpp\n> > > @@ -62,7 +62,7 @@ void EventLoop::interrupt()\n> > >  \n> > >  void EventLoop::callLater(const std::function<void()> &func)\n> > >  {\n> > > -\t{\n> > > +\tif (!exit_.load(std::memory_order_acquire)) {\n> > >  \t\tstd::unique_lock<std::mutex> locker(lock_);\n> > >  \t\tcalls_.push_back(func);\n> > >  \t}\n> > \n> > That's a bit of a layering violation. Would it make sense to fix this\n> > differently, by ensuring that EventLoop::dispatchCalls() will only\n> > handle deferred calls that are on the list when the function is called ?\n> > This would also ensure that callLater(), when called from the same\n> > thread, will guarantee that pending events get processed before the\n> > delayed call is processed.\n> > \n> > I mean something along those lines (only compile-tested).\n> > \n> > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp\n> > index 94c5d1d36245..320ade525a3f 100644\n> > --- a/src/cam/event_loop.cpp\n> > +++ b/src/cam/event_loop.cpp\n> > @@ -74,7 +74,7 @@ void EventLoop::dispatchCalls()\n> >  {\n> >  \tstd::unique_lock<std::mutex> locker(lock_);\n> > \n> > -\tfor (auto iter = calls_.begin(); iter != calls_.end(); ) {\n> > +\tfor (auto iter = calls_.begin(), end = calls_.end(); iter != end; ) {\n> >  \t\tstd::function<void()> call = std::move(*iter);\n> > \n> >  \t\titer = calls_.erase(iter);\n> \n> I tried something like this but thought it was nastier then I proposed \n> in this patch. I do however agree that what is proposed in my patch is \n> also not the most cleanest of solutions. The change you propose here \n> does not work as intended, it is still venerable to an ever increasing \n> call_ vector.\n> \n> If we would like to go with this solution how about something like this?  \n> It would also reduce the lock_ contention as a side effect.\n> \n> diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp\n> index 94c5d1d362455f33..4bfcc503ac1f3b83 100644\n> --- a/src/cam/event_loop.cpp\n> +++ b/src/cam/event_loop.cpp\n> @@ -72,15 +72,13 @@ void EventLoop::callLater(const std::function<void()> &func)\n>  \n>  void EventLoop::dispatchCalls()\n>  {\n> -       std::unique_lock<std::mutex> locker(lock_);\n> +       std::list<std::function<void()>> calls;\n>  \n> -       for (auto iter = calls_.begin(); iter != calls_.end(); ) {\n> -               std::function<void()> call = std::move(*iter);\n> +       {\n> +               std::unique_lock<std::mutex> locker(lock_);\n> +               calls = std::move(calls_);\n> +       }\n>  \n> -               iter = calls_.erase(iter);\n> -\n> -               locker.unlock();\n> +       for (std::function<void()> call : calls)\n>                 call();\n> -               locker.lock();\n> -       }\n>  }\n\nI Think this back as both solutions that modifies dispatchCalls() \nsuffers from the same flaw. Events queued before the EventLoop::exit() \nare not executed if they are not already part of the active list of \ncalls that are processed.\n\nWhat do you think of injecting a 'end of call processing' marker into \nthe calls_ vector and break the loop in dispatchCalls() if it's \nencounter? Spontaneously I'm thinking of a 'nullptr' marker, but it's \nuntested.\n\n> \n> > \n> > \n> > -- \n> > Regards,\n> > \n> > Laurent Pinchart\n> \n> -- \n> Regards,\n> Niklas Söderlund","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 E59A6C0F1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Jan 2021 14:07:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 75D676814B;\n\tTue, 19 Jan 2021 15:07:19 +0100 (CET)","from mail-lj1-x22e.google.com (mail-lj1-x22e.google.com\n\t[IPv6:2a00:1450:4864:20::22e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5C7F968140\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Jan 2021 15:07:18 +0100 (CET)","by mail-lj1-x22e.google.com with SMTP id n8so13229344ljg.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Jan 2021 06:07:18 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tq21sm2364800lfc.290.2021.01.19.06.07.16\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 19 Jan 2021 06:07:16 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"npNENYpj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=sij12cDJnFKe9XVXU5iKsUdSbZ+LjFniAA+PCJlAnd0=;\n\tb=npNENYpjjTqNwEg1UxHgNMNp++8iLBsvo5zno9sPQlrHXA9d5Pd+XRazS1wBZScLiI\n\ts6KPQUoz4r+s14ivx7BENTL74ax0EoB16j9ww9YQtg0mE08KR9HMuX5bpBz1m4tb/T03\n\tm68iy2RVmmg0gb6Bfxp1X8evcDBfw7EeIl2bTvwmeBoVI7XxEzfKQg+MqhSQdw2Nbrf4\n\tuSaEn00JdVADDNO24qMaDRDoI3Ak6ZDt08kqM5u8MfdcT6gMvjgEB+FqaMWgmOLC2ziV\n\ttUjg1LY3eFba6YDTKuBJfTSt0YFiuOV/CQDlq56/QqT3124+Rg4RDvmA+Qr6WuTugg+j\n\tRoYQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=sij12cDJnFKe9XVXU5iKsUdSbZ+LjFniAA+PCJlAnd0=;\n\tb=FgReJG7gmrdLN6vITQIBYOwa/PW+rIvGozuz+PEWa+ZPkKTWhEIGDcpvKIZyluaYTJ\n\tfBzw4VI2HcYD/Ldr0NFiENqNbvp7w63fGm5QRHnU0e5quMT8TcWbsXeG77idQnslbLZ7\n\ta2QEEsyRbO7ygtiqz4rhC3cJ4GHYpnSFIvFp0M1O1Xvw7VtVQF22Zc7Suth3Ddnloibn\n\tGr2QGAvc43goEL5OpD2/Z8qCBRumTxD4lA9FCby8bJvhUIK1LOnDjfCGt2imp5HApSvw\n\teGU0V4tdfho/mguz8jfl910CCEjGceSbplRWFhzO/M5SrG6ZkASrDEg8UDaJPSzDpnQS\n\tm4gA==","X-Gm-Message-State":"AOAM530gngcpI3vtdjc000umHLGpP/m1BNIRC4bqYTjszXbxRyLzJMTL\n\tjhXhh4h+PJSRXiATUioRAWJY7t5WY2PmSg==","X-Google-Smtp-Source":"ABdhPJwDsWzYZ55d6Sb+mBXzC7G9TJJrLupNQGKyGN3GX3aNFmcyVBTEeDjaMrjw9tS+L/HwbmOHcw==","X-Received":"by 2002:a05:651c:283:: with SMTP id\n\tb3mr2010830ljo.345.1611065237740; \n\tTue, 19 Jan 2021 06:07:17 -0800 (PST)","Date":"Tue, 19 Jan 2021 15:07:16 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<YAbnlFx/FoS0BkgC@oden.dyn.berto.se>","References":"<20210119123110.2976971-1-niklas.soderlund@ragnatech.se>\n\t<20210119123110.2976971-2-niklas.soderlund@ragnatech.se>\n\t<YAbT5XWeuqOLIxbQ@pendragon.ideasonboard.com>\n\t<YAbhwdkOljA3SGSt@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<YAbhwdkOljA3SGSt@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH 1/2] cam: event_loop: Stop queuing\n\tcalls when the event loop are exiting","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14607,"web_url":"https://patchwork.libcamera.org/comment/14607/","msgid":"<YAcC/tOm7KrilkZy@pendragon.ideasonboard.com>","date":"2021-01-19T16:04:14","subject":"Re: [libcamera-devel] [PATCH 1/2] cam: event_loop: Stop queuing\n\tcalls when the event loop are exiting","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Tue, Jan 19, 2021 at 03:07:16PM +0100, Niklas Söderlund wrote:\n> On 2021-01-19 14:42:27 +0100, Niklas Söderlund wrote:\n> > On 2021-01-19 14:43:17 +0200, Laurent Pinchart wrote:\n> > > On Tue, Jan 19, 2021 at 01:31:09PM +0100, Niklas Söderlund wrote:\n> > > > Terminating the event loop with EventLoop::exit() does not grantee that\n> > > > it will terminate. If the event loops 'call later' queue can be kept\n> > > > non-empty by continuously queuing new calls using EventLoop::callLater()\n> > > > either from a different thread or from callbacks in the loop itself\n> > > > EventLoop::dispatchCalls() will never complete and the loop will run\n> > > > forever.\n> > > > \n> > > > Solve this by not accepting new callbacks once the event loop is\n> > > > exiting.\n> > > > \n> > > > Reported-by: Sebastian Fricke <sebastian.fricke@posteo.net>\n> > > > Fixes: f49e93338b6309a6 (\"cam: event_loop: Add deferred calls support\")\n> > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > > ---\n> > > >  src/cam/event_loop.cpp | 2 +-\n> > > >  1 file changed, 1 insertion(+), 1 deletion(-)\n> > > > \n> > > > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp\n> > > > index 94c5d1d362455f33..34a55da30c537ac7 100644\n> > > > --- a/src/cam/event_loop.cpp\n> > > > +++ b/src/cam/event_loop.cpp\n> > > > @@ -62,7 +62,7 @@ void EventLoop::interrupt()\n> > > >  \n> > > >  void EventLoop::callLater(const std::function<void()> &func)\n> > > >  {\n> > > > -\t{\n> > > > +\tif (!exit_.load(std::memory_order_acquire)) {\n> > > >  \t\tstd::unique_lock<std::mutex> locker(lock_);\n> > > >  \t\tcalls_.push_back(func);\n> > > >  \t}\n> > > \n> > > That's a bit of a layering violation. Would it make sense to fix this\n> > > differently, by ensuring that EventLoop::dispatchCalls() will only\n> > > handle deferred calls that are on the list when the function is called ?\n> > > This would also ensure that callLater(), when called from the same\n> > > thread, will guarantee that pending events get processed before the\n> > > delayed call is processed.\n> > > \n> > > I mean something along those lines (only compile-tested).\n> > > \n> > > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp\n> > > index 94c5d1d36245..320ade525a3f 100644\n> > > --- a/src/cam/event_loop.cpp\n> > > +++ b/src/cam/event_loop.cpp\n> > > @@ -74,7 +74,7 @@ void EventLoop::dispatchCalls()\n> > >  {\n> > >  \tstd::unique_lock<std::mutex> locker(lock_);\n> > > \n> > > -\tfor (auto iter = calls_.begin(); iter != calls_.end(); ) {\n> > > +\tfor (auto iter = calls_.begin(), end = calls_.end(); iter != end; ) {\n> > >  \t\tstd::function<void()> call = std::move(*iter);\n> > > \n> > >  \t\titer = calls_.erase(iter);\n> > \n> > I tried something like this but thought it was nastier then I proposed \n> > in this patch. I do however agree that what is proposed in my patch is \n> > also not the most cleanest of solutions. The change you propose here \n> > does not work as intended, it is still venerable to an ever increasing \n> > call_ vector.\n> > \n> > If we would like to go with this solution how about something like this?  \n> > It would also reduce the lock_ contention as a side effect.\n> > \n> > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp\n> > index 94c5d1d362455f33..4bfcc503ac1f3b83 100644\n> > --- a/src/cam/event_loop.cpp\n> > +++ b/src/cam/event_loop.cpp\n> > @@ -72,15 +72,13 @@ void EventLoop::callLater(const std::function<void()> &func)\n> >  \n> >  void EventLoop::dispatchCalls()\n> >  {\n> > -       std::unique_lock<std::mutex> locker(lock_);\n> > +       std::list<std::function<void()>> calls;\n> >  \n> > -       for (auto iter = calls_.begin(); iter != calls_.end(); ) {\n> > -               std::function<void()> call = std::move(*iter);\n> > +       {\n> > +               std::unique_lock<std::mutex> locker(lock_);\n> > +               calls = std::move(calls_);\n> > +       }\n> >  \n> > -               iter = calls_.erase(iter);\n> > -\n> > -               locker.unlock();\n> > +       for (std::function<void()> call : calls)\n> >                 call();\n> > -               locker.lock();\n> > -       }\n> >  }\n\nThat looks good too.\n\n> I Think this back as both solutions that modifies dispatchCalls() \n> suffers from the same flaw. Events queued before the EventLoop::exit() \n> are not executed if they are not already part of the active list of \n> calls that are processed.\n\nIs that a problem though ? If we need to ensure that the deferred calls\nqueue is flushed, maybe we could make dispatchCalls() public and call it\nafter EventLoop::exit() returns ?\n\n> What do you think of injecting a 'end of call processing' marker into \n> the calls_ vector and break the loop in dispatchCalls() if it's \n> encounter? Spontaneously I'm thinking of a 'nullptr' marker, but it's \n> untested.\n\nWhat's the problem that needs to be solved here ? There's an inherent\nrace condition if we queue calls from a different thread, how to handle\nit best depends on what the use case is.","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 6F29DBD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Jan 2021 16:04:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EE7996814E;\n\tTue, 19 Jan 2021 17:04:33 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 118E16814E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Jan 2021 17:04:32 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 90427813;\n\tTue, 19 Jan 2021 17:04:31 +0100 (CET)"],"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=\"wcZT5fLK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611072271;\n\tbh=ce5JrL6DQnUW6aUhoIOwNgU7CstmIrAYSAPmgf2HZOo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=wcZT5fLKdl/IrnATBRZcaw+CvjTG1liqvxiFIrCIVow8WWq6E/1YKQSwTr0wEGGIX\n\tazJYP4bnNKyOp5hmmN9r3mpEFTVIzs63t/Pe0Jf1J4pFpkjmEkgEYLFnlWsc5Se6Ly\n\tz2mA6i2Hu54skm/YQ+fYqdiFCENuVyweeLgaPqWo=","Date":"Tue, 19 Jan 2021 18:04:14 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<YAcC/tOm7KrilkZy@pendragon.ideasonboard.com>","References":"<20210119123110.2976971-1-niklas.soderlund@ragnatech.se>\n\t<20210119123110.2976971-2-niklas.soderlund@ragnatech.se>\n\t<YAbT5XWeuqOLIxbQ@pendragon.ideasonboard.com>\n\t<YAbhwdkOljA3SGSt@oden.dyn.berto.se>\n\t<YAbnlFx/FoS0BkgC@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<YAbnlFx/FoS0BkgC@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH 1/2] cam: event_loop: Stop queuing\n\tcalls when the event loop are exiting","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14617,"web_url":"https://patchwork.libcamera.org/comment/14617/","msgid":"<YAdsI9WVDvwnxCJB@oden.dyn.berto.se>","date":"2021-01-19T23:32:51","subject":"Re: [libcamera-devel] [PATCH 1/2] cam: event_loop: Stop queuing\n\tcalls when the event loop are exiting","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nOn 2021-01-19 18:04:14 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Tue, Jan 19, 2021 at 03:07:16PM +0100, Niklas Söderlund wrote:\n> > On 2021-01-19 14:42:27 +0100, Niklas Söderlund wrote:\n> > > On 2021-01-19 14:43:17 +0200, Laurent Pinchart wrote:\n> > > > On Tue, Jan 19, 2021 at 01:31:09PM +0100, Niklas Söderlund wrote:\n> > > > > Terminating the event loop with EventLoop::exit() does not grantee that\n> > > > > it will terminate. If the event loops 'call later' queue can be kept\n> > > > > non-empty by continuously queuing new calls using EventLoop::callLater()\n> > > > > either from a different thread or from callbacks in the loop itself\n> > > > > EventLoop::dispatchCalls() will never complete and the loop will run\n> > > > > forever.\n> > > > > \n> > > > > Solve this by not accepting new callbacks once the event loop is\n> > > > > exiting.\n> > > > > \n> > > > > Reported-by: Sebastian Fricke <sebastian.fricke@posteo.net>\n> > > > > Fixes: f49e93338b6309a6 (\"cam: event_loop: Add deferred calls support\")\n> > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > > > ---\n> > > > >  src/cam/event_loop.cpp | 2 +-\n> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)\n> > > > > \n> > > > > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp\n> > > > > index 94c5d1d362455f33..34a55da30c537ac7 100644\n> > > > > --- a/src/cam/event_loop.cpp\n> > > > > +++ b/src/cam/event_loop.cpp\n> > > > > @@ -62,7 +62,7 @@ void EventLoop::interrupt()\n> > > > >  \n> > > > >  void EventLoop::callLater(const std::function<void()> &func)\n> > > > >  {\n> > > > > -\t{\n> > > > > +\tif (!exit_.load(std::memory_order_acquire)) {\n> > > > >  \t\tstd::unique_lock<std::mutex> locker(lock_);\n> > > > >  \t\tcalls_.push_back(func);\n> > > > >  \t}\n> > > > \n> > > > That's a bit of a layering violation. Would it make sense to fix this\n> > > > differently, by ensuring that EventLoop::dispatchCalls() will only\n> > > > handle deferred calls that are on the list when the function is called ?\n> > > > This would also ensure that callLater(), when called from the same\n> > > > thread, will guarantee that pending events get processed before the\n> > > > delayed call is processed.\n> > > > \n> > > > I mean something along those lines (only compile-tested).\n> > > > \n> > > > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp\n> > > > index 94c5d1d36245..320ade525a3f 100644\n> > > > --- a/src/cam/event_loop.cpp\n> > > > +++ b/src/cam/event_loop.cpp\n> > > > @@ -74,7 +74,7 @@ void EventLoop::dispatchCalls()\n> > > >  {\n> > > >  \tstd::unique_lock<std::mutex> locker(lock_);\n> > > > \n> > > > -\tfor (auto iter = calls_.begin(); iter != calls_.end(); ) {\n> > > > +\tfor (auto iter = calls_.begin(), end = calls_.end(); iter != end; ) {\n> > > >  \t\tstd::function<void()> call = std::move(*iter);\n> > > > \n> > > >  \t\titer = calls_.erase(iter);\n> > > \n> > > I tried something like this but thought it was nastier then I proposed \n> > > in this patch. I do however agree that what is proposed in my patch is \n> > > also not the most cleanest of solutions. The change you propose here \n> > > does not work as intended, it is still venerable to an ever increasing \n> > > call_ vector.\n> > > \n> > > If we would like to go with this solution how about something like this?  \n> > > It would also reduce the lock_ contention as a side effect.\n> > > \n> > > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp\n> > > index 94c5d1d362455f33..4bfcc503ac1f3b83 100644\n> > > --- a/src/cam/event_loop.cpp\n> > > +++ b/src/cam/event_loop.cpp\n> > > @@ -72,15 +72,13 @@ void EventLoop::callLater(const std::function<void()> &func)\n> > >  \n> > >  void EventLoop::dispatchCalls()\n> > >  {\n> > > -       std::unique_lock<std::mutex> locker(lock_);\n> > > +       std::list<std::function<void()>> calls;\n> > >  \n> > > -       for (auto iter = calls_.begin(); iter != calls_.end(); ) {\n> > > -               std::function<void()> call = std::move(*iter);\n> > > +       {\n> > > +               std::unique_lock<std::mutex> locker(lock_);\n> > > +               calls = std::move(calls_);\n> > > +       }\n> > >  \n> > > -               iter = calls_.erase(iter);\n> > > -\n> > > -               locker.unlock();\n> > > +       for (std::function<void()> call : calls)\n> > >                 call();\n> > > -               locker.lock();\n> > > -       }\n> > >  }\n> \n> That looks good too.\n> \n> > I Think this back as both solutions that modifies dispatchCalls() \n> > suffers from the same flaw. Events queued before the EventLoop::exit() \n> > are not executed if they are not already part of the active list of \n> > calls that are processed.\n> \n> Is that a problem though ? If we need to ensure that the deferred calls\n> queue is flushed, maybe we could make dispatchCalls() public and call it\n> after EventLoop::exit() returns ?\n> \n> > What do you think of injecting a 'end of call processing' marker into \n> > the calls_ vector and break the loop in dispatchCalls() if it's \n> > encounter? Spontaneously I'm thinking of a 'nullptr' marker, but it's \n> > untested.\n> \n> What's the problem that needs to be solved here ? There's an inherent\n> race condition if we queue calls from a different thread, how to handle\n> it best depends on what the use case is.\n\nI agree the proper solution is likely what best matches our use-case. I \nposted a v2 of this series (it's now just a single patch) that is the \nminimal change to get the behavior back to what is before the deferred \ncall support.\n\nI'm sure we can do better on the over all goal of only queuing N \nrequests instead of terminating when we are done. But that would be a \nlarger change and right now I just want to get back to deterministic and \nfunctional behavior :-) Over all I think we soon will need a compliance \ntool to verify pipeline behavior to be able to make our other tools more \nstrict.\n\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 EDB9CC0F1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Jan 2021 23:32:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5B09768170;\n\tWed, 20 Jan 2021 00:32:56 +0100 (CET)","from mail-lf1-x129.google.com (mail-lf1-x129.google.com\n\t[IPv6:2a00:1450:4864:20::129])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 204176010B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 20 Jan 2021 00:32:54 +0100 (CET)","by mail-lf1-x129.google.com with SMTP id 23so31562996lfg.10\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Jan 2021 15:32:54 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id f21sm31190lfe.6.2021.01.19.15.32.52\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 19 Jan 2021 15:32:52 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"A1w1shwf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=zzk4ETcRNgogReoGBBHbNXZhGUrH8Ha5Ot8hCfHmuXU=;\n\tb=A1w1shwfBWzpbK3bhxaAS28skLC8dxZ59y1ACNYayEFHJC4Euxjgcja33Dil7mH+FU\n\tmbXJew2ggtL+gFgSgktZmY4LByTuySENXLS2sRm1unylzRakFZ/eYw4CHy+7Z9QF+xW/\n\t0v6+TCy2+sgKJ9LCV3ytTMbU2ea0JjKSpzC6vkRwIcfcBYaTFqC/lWKzIxwXpeO/FABJ\n\to4TyBQUjhqC+SdG+f5R7/r+6JEsVQQJSsZKXg4cJhsiJEnL97+NSq6Qdjnr21rpFr4oP\n\twq9Ao506YkShK2N/oDPNFeNmxt+E7p1JK5kAmimdOzLXlECBzinTxd6oxDBW8PRBE3HA\n\t6P+w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=zzk4ETcRNgogReoGBBHbNXZhGUrH8Ha5Ot8hCfHmuXU=;\n\tb=K7OG+rK9DUpAbeV2hD3hhjR/EbwOO9V8C7ctCKfqJQjacw48uEDqt8q2KEbqy1QhAZ\n\tbmp2F6fJCwH0Lt1Xml8QWPhVl6ZcW8vxZHxEnQApMFzwny5XDXdZcqUcBmZwM2g5udrE\n\t5rsdPiv6x7X7j7hktqtXkqCG78Xwd3/kvxRCzM9+WpZD5uJIr9okNddNLLjB/dKd1O2x\n\tbbMt4SosFGdubFITO5vYaAvQxyPCF0Ml1Hyl845fpt+HtqnxB08o2clgYmNle66RsyDI\n\tLiAyhH8iouKCaoC+McO/pk3sjAatBiqGf5L7s/uPAClvrShkAh02Nn/Tboqi2z0+MoFz\n\twbQA==","X-Gm-Message-State":"AOAM530AViAoV19YAOfkUoEpFKtKI4Dcykt50mVnBQM9kdEdI9q98GID\n\tVVrMGm5d4847WwD8kQ5trRUCuQ==","X-Google-Smtp-Source":"ABdhPJyjtsfnwISUkGm4zyos5A8iC3UWjHDnBrfJ4pk+bIZFddmQZpFw5D1Czi9oKC5+3hvUA5BrFQ==","X-Received":"by 2002:ac2:4561:: with SMTP id k1mr1122748lfm.70.1611099173277; \n\tTue, 19 Jan 2021 15:32:53 -0800 (PST)","Date":"Wed, 20 Jan 2021 00:32:51 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<YAdsI9WVDvwnxCJB@oden.dyn.berto.se>","References":"<20210119123110.2976971-1-niklas.soderlund@ragnatech.se>\n\t<20210119123110.2976971-2-niklas.soderlund@ragnatech.se>\n\t<YAbT5XWeuqOLIxbQ@pendragon.ideasonboard.com>\n\t<YAbhwdkOljA3SGSt@oden.dyn.berto.se>\n\t<YAbnlFx/FoS0BkgC@oden.dyn.berto.se>\n\t<YAcC/tOm7KrilkZy@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<YAcC/tOm7KrilkZy@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/2] cam: event_loop: Stop queuing\n\tcalls when the event loop are exiting","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]