[{"id":3157,"web_url":"https://patchwork.libcamera.org/comment/3157/","msgid":"<20191127151036.GI12879@bigcity.dyn.berto.se>","date":"2019-11-27T15:10:36","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: object: Document\n\tdanger of deleting object from other thread","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 work.\n\nOn 2019-11-27 10:49:08 +0200, Laurent Pinchart wrote:\n> Object instances receive messages dispatched from the event loop of the\n> thread they belong to. Deleting an object from a different thread is\n> thus dangerous, unless the caller ensures that no message delivery is in\n> progress. Document this in the Object class documentation.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> src/libcamera/object.cpp | 4 ++++\n> 1 file changed, 4 insertions(+)\n> \n> diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp\n> index 1f787271f782..e76faf48b8ed 100644\n> --- a/src/libcamera/object.cpp\n> +++ b/src/libcamera/object.cpp\n> @@ -40,6 +40,10 @@ LOG_DEFINE_CATEGORY(Object)\n> * implementing easy message passing between threads by inheriting from the\n> * Object class.\n> *\n> + * Deleting an object from a thread other than the one the object is bound to is\n> + * unsafe, unless the caller ensures that the object isn't processing any\n> + * message concurrently.\n> + *\n> * Object slots connected to signals will also run in the context of the\n> * object's thread, regardless of whether the signal is emitted in the same or\n> * in another thread.\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 965A060C33\n\tfor ;\n\tWed, 27 Nov 2019 16:10:38 +0100 (CET)","by mail-lj1-x241.google.com with SMTP id n21so24842006ljg.12\n\tfor ;\n\tWed, 27 Nov 2019 07:10:38 -0800 (PST)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tn83sm7237240lfd.70.2019.11.27.07.10.37\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 27 Nov 2019 07:10:37 -0800 (PST)"],"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\t:user-agent; bh=k5QUiOjKHz9I4EtfUNS87OlYPSfM1DJYVmI0elBApa4=;\n\tb=WbqhBU8SR+bBA/9iIemTACM05F+WDoTnReJnW/PM+b6m0cnd05jxPfySTvkq0iSq3F\n\tRXYmluut6VySFLGh8qHLVqecl0mszfdqba2J9ktt1BqDiPYeuearDemL3a99kNMX5Iwl\n\tZGKMe0jyaV8HB7VuePNupH1ase1veEDvdTdyKbVNHU7VNIKPbF7Hku9eWupAWT0f7n/Y\n\tUENhn/FBYDUtG2DMxJQZh/adPOFrL3AjCFWO2RlQqI3V0MJAHTLC5BuI6pbJ5KlJtS2Z\n\tiqAy4zOr8aQBJymUGTGyDehEafSuoaKhAoaQ4+MhXSN468kgj04fhnXaKI9PmsjwAini\n\t9rvA==","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:user-agent;\n\tbh=k5QUiOjKHz9I4EtfUNS87OlYPSfM1DJYVmI0elBApa4=;\n\tb=gNZhvJGUiVuUqjsgQuyoy/3wykSwBQhJBHxEZ929IbkLBiOx3DQ0Zmxn91+Sp3rZor\n\t3G3aFPVK+fg27dkpO9nEMQvafo1eAFJySKCEdK2xDjzUVKGBOtxNVbAupWY0Q9ykTboL\n\tjG40P7ZbnspikxocGzBJJKdNv42087cBXP+6t+1nerJ6mM9latuz9x6NhczshWfssRma\n\tP66AprFwat9Le9azdBlsufT6U3mwjkSDspTXZy47Bull9YCkL5GpOt4tH51z+MFizO9I\n\tkhbajD9XxDooFwM0OsVADrfkWYYfzRrz45kY/kcMPrry0HFu+jDSgmBtHC+n8hz0JffP\n\t/sEg==","X-Gm-Message-State":"APjAAAXtnQP2TTOvLBQBmzkrsvnb6F3oPK4qa2AzoTxcrjNhcVpmQgY2\n\th7pWONnyMxCIVqAuBwRLvtbJuw==","X-Google-Smtp-Source":"APXvYqyoutdUTn8VmFy+AlXjoaTeZ8mtBOs69DkTFbeTGWFEFDQGV3UpzgX1lWZcpfNc5tGFWMwcnA==","X-Received":"by 2002:a2e:91d5:: with SMTP id\n\tu21mr31773295ljg.32.1574867437864; \n\tWed, 27 Nov 2019 07:10:37 -0800 (PST)","Date":"Wed, 27 Nov 2019 16:10:36 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191127151036.GI12879@bigcity.dyn.berto.se>","References":"<20191127084909.10612-1-laurent.pinchart@ideasonboard.com>\n\t<20191127084909.10612-4-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191127084909.10612-4-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: object: Document\n\tdanger of deleting object from other thread","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 27 Nov 2019 15:10:38 -0000"}},{"id":3158,"web_url":"https://patchwork.libcamera.org/comment/3158/","msgid":"<8921f9df-490d-6569-bf41-e00907d2b146@ideasonboard.com>","date":"2019-11-27T15:14:08","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: object: Document\n\tdanger of deleting object from other thread","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 27/11/2019 08:49, Laurent Pinchart wrote:\n> Object instances receive messages dispatched from the event loop of the\n> thread they belong to. Deleting an object from a different thread is\n> thus dangerous, unless the caller ensures that no message delivery is in\n> progress. Document this in the Object class documentation.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/object.cpp | 4 ++++\n> 1 file changed, 4 insertions(+)\n> \n> diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp\n> index 1f787271f782..e76faf48b8ed 100644\n> --- a/src/libcamera/object.cpp\n> +++ b/src/libcamera/object.cpp\n> @@ -40,6 +40,10 @@ LOG_DEFINE_CATEGORY(Object)\n> * implementing easy message passing between threads by inheriting from the\n> * Object class.\n> *\n> + * Deleting an object from a thread other than the one the object is bound to is\n> + * unsafe, unless the caller ensures that the object isn't processing any\n> + * message concurrently.\n> + *\n\nWhat about codifying this with an assertion or Log(Error) check?\n\nI.e. put something into the destructor which tests the current thread\nagainst the object bound thread?\n\nI think a runtime check here would be cheap, and serve a better way to\ncatch these bugs than simply documenting potential issues.\n\n> * Object slots connected to signals will also run in the context of the\n> * object's thread, regardless of whether the signal is emitted in the same or\n> * in another thread.\n>","headers":{"Return-Path":"","Received":["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 4EB8460C33\n\tfor ;\n\tWed, 27 Nov 2019 16:14:12 +0100 (CET)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A1390325;\n\tWed, 27 Nov 2019 16:14:11 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1574867651;\n\tbh=d3iITQCle8ltfwTobxo4jYBiaxKOIqtoMFFDN57JZTk=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=sRLnqFI3YRFoLUHmohGQkZEmDF+A3phZiqMLQzHIqSxHAliD3sl8vsUK4K80MNPtx\n\tLrZlyamBsfA1LDKr1hRgkXjBc/GazHyRgoL686Rwtwv7AtD0EN438YEbuwL9mGty6Q\n\t/XVUTHCYkj/CU+girDCRc+UfExhk7AgQTAWe8ttw=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20191127084909.10612-1-laurent.pinchart@ideasonboard.com>\n\t<20191127084909.10612-4-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<8921f9df-490d-6569-bf41-e00907d2b146@ideasonboard.com>","Date":"Wed, 27 Nov 2019 15:14:08 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.9.0","MIME-Version":"1.0","In-Reply-To":"<20191127084909.10612-4-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: object: Document\n\tdanger of deleting object from other thread","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 27 Nov 2019 15:14:12 -0000"}},{"id":3160,"web_url":"https://patchwork.libcamera.org/comment/3160/","msgid":"<20191127151811.GC4730@pendragon.ideasonboard.com>","date":"2019-11-27T15:20:49","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: object: Document\n\tdanger of deleting object from other thread","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Wed, Nov 27, 2019 at 03:14:08PM +0000, Kieran Bingham wrote:\n> On 27/11/2019 08:49, Laurent Pinchart wrote:\n> > Object instances receive messages dispatched from the event loop of the\n> > thread they belong to. Deleting an object from a different thread is\n> > thus dangerous, unless the caller ensures that no message delivery is in\n> > progress. Document this in the Object class documentation.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/object.cpp | 4 ++++\n> > 1 file changed, 4 insertions(+)\n> > \n> > diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp\n> > index 1f787271f782..e76faf48b8ed 100644\n> > --- a/src/libcamera/object.cpp\n> > +++ b/src/libcamera/object.cpp\n> > @@ -40,6 +40,10 @@ LOG_DEFINE_CATEGORY(Object)\n> > * implementing easy message passing between threads by inheriting from the\n> > * Object class.\n> > *\n> > + * Deleting an object from a thread other than the one the object is bound to is\n> > + * unsafe, unless the caller ensures that the object isn't processing any\n> > + * message concurrently.\n> > + *\n> \n> What about codifying this with an assertion or Log(Error) check?\n> \n> I.e. put something into the destructor which tests the current thread\n> against the object bound thread?\n> \n> I think a runtime check here would be cheap, and serve a better way to\n> catch these bugs than simply documenting potential issues.\n\nI'd do so if it wasn't for the \"unless the caller\" part. It's valid to\ndestroy objects from other threads if we're careful, and one simple use\ncase is destroying objects that have been moved to a thread after the\nthread finishes.\n\n> > * Object slots connected to signals will also run in the context of the\n> > * object's thread, regardless of whether the signal is emitted in the same or\n> > * in another thread.","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A2AB60C33\n\tfor ;\n\tWed, 27 Nov 2019 16:21:01 +0100 (CET)","from pendragon.ideasonboard.com (fs96f9c64d.tkyc509.ap.nuro.jp\n\t[150.249.198.77])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9FA50325;\n\tWed, 27 Nov 2019 16:20:59 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1574868060;\n\tbh=9SgA1oJ20tiBG2R3RLAw5O7tlywQyKZiOKqJDIup3jI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NPicuJjTbVDDWFXUqYyPYQDuMUW9V9VeI09x9n8cieSKhvOMbYzT6eZLJCYo00zOS\n\tTEytXJ67xuhZqAgJWTPOGeMNWajP5d+5//sC/RAUJjSWu3qRBTbfEOqUaTSOa6XJGD\n\t03E4uXV66SnEsp/j6dVOqpIi6/2s9J2MnDjRromQ=","Date":"Wed, 27 Nov 2019 17:20:49 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191127151811.GC4730@pendragon.ideasonboard.com>","References":"<20191127084909.10612-1-laurent.pinchart@ideasonboard.com>\n\t<20191127084909.10612-4-laurent.pinchart@ideasonboard.com>\n\t<8921f9df-490d-6569-bf41-e00907d2b146@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<8921f9df-490d-6569-bf41-e00907d2b146@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: object: Document\n\tdanger of deleting object from other thread","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 27 Nov 2019 15:21:01 -0000"}},{"id":3161,"web_url":"https://patchwork.libcamera.org/comment/3161/","msgid":"<2841d4d1-b58d-cd07-f8b9-e071c027ce58@ideasonboard.com>","date":"2019-11-27T15:42:29","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: object: Document\n\tdanger of deleting object from other thread","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 27/11/2019 15:20, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Wed, Nov 27, 2019 at 03:14:08PM +0000, Kieran Bingham wrote:\n>> On 27/11/2019 08:49, Laurent Pinchart wrote:\n>>> Object instances receive messages dispatched from the event loop of the\n>>> thread they belong to. Deleting an object from a different thread is\n>>> thus dangerous, unless the caller ensures that no message delivery is in\n>>> progress. Document this in the Object class documentation.\n>>>\n>>> Signed-off-by: Laurent Pinchart \n>>> ---\n>>> src/libcamera/object.cpp | 4 ++++\n>>> 1 file changed, 4 insertions(+)\n>>>\n>>> diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp\n>>> index 1f787271f782..e76faf48b8ed 100644\n>>> --- a/src/libcamera/object.cpp\n>>> +++ b/src/libcamera/object.cpp\n>>> @@ -40,6 +40,10 @@ LOG_DEFINE_CATEGORY(Object)\n>>> * implementing easy message passing between threads by inheriting from the\n>>> * Object class.\n>>> *\n>>> + * Deleting an object from a thread other than the one the object is bound to is\n>>> + * unsafe, unless the caller ensures that the object isn't processing any\n>>> + * message concurrently.\n>>> + *\n>>\n>> What about codifying this with an assertion or Log(Error) check?\n>>\n>> I.e. put something into the destructor which tests the current thread\n>> against the object bound thread?\n>>\n>> I think a runtime check here would be cheap, and serve a better way to\n>> catch these bugs than simply documenting potential issues.\n> \n> I'd do so if it wasn't for the \"unless the caller\" part. It's valid to\n> destroy objects from other threads if we're careful, and one simple use\n> case is destroying objects that have been moved to a thread after the\n> thread finishes.\n\nBut the message says \n \"From an object if the object is processing a message\"\n\nSo doesn't that make the check (psuedo-code):\n\n if (pendingMessages_ && this_thread != object->bound_thread())\n\tLog(Error)\n\t << \"Destroying object from separate thread with messages\";\n\n\nOr maybe I've misunderstood something?\n\n--\nKieran\n\n> \n>>> * Object slots connected to signals will also run in the context of the\n>>> * object's thread, regardless of whether the signal is emitted in the same or\n>>> * in another thread.\n>","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0206460C33\n\tfor ;\n\tWed, 27 Nov 2019 16:42:33 +0100 (CET)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5BBBC325;\n\tWed, 27 Nov 2019 16:42:33 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1574869353;\n\tbh=ZyWh9nHusNDcUv0d48JW8MQaaM1UIksRkUSw5lctfMQ=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=W0b1RuH00QT+JqipbMUGCd56Sy4Tfa5YlOa7I+Lw8wcTFvXITLsagS/Il/QenpIdM\n\t9gKayKKyiGMNb9Lu2JWXxfrnvZbkq1OK5uNBm8wtk70aOsfW/lRb2nKhwIxQGg1klY\n\tUc7g2YEA2dSBeBSXfM5HytztXFM5y/eQwMLi1hG4=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20191127084909.10612-1-laurent.pinchart@ideasonboard.com>\n\t<20191127084909.10612-4-laurent.pinchart@ideasonboard.com>\n\t<8921f9df-490d-6569-bf41-e00907d2b146@ideasonboard.com>\n\t<20191127151811.GC4730@pendragon.ideasonboard.com>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<2841d4d1-b58d-cd07-f8b9-e071c027ce58@ideasonboard.com>","Date":"Wed, 27 Nov 2019 15:42:29 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.9.0","MIME-Version":"1.0","In-Reply-To":"<20191127151811.GC4730@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: object: Document\n\tdanger of deleting object from other thread","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 27 Nov 2019 15:42:34 -0000"}},{"id":3162,"web_url":"https://patchwork.libcamera.org/comment/3162/","msgid":"<20191127160109.GD4730@pendragon.ideasonboard.com>","date":"2019-11-27T16:01:09","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: object: Document\n\tdanger of deleting object from other thread","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Wed, Nov 27, 2019 at 03:42:29PM +0000, Kieran Bingham wrote:\n> On 27/11/2019 15:20, Laurent Pinchart wrote:\n> > On Wed, Nov 27, 2019 at 03:14:08PM +0000, Kieran Bingham wrote:\n> >> On 27/11/2019 08:49, Laurent Pinchart wrote:\n> >>> Object instances receive messages dispatched from the event loop of the\n> >>> thread they belong to. Deleting an object from a different thread is\n> >>> thus dangerous, unless the caller ensures that no message delivery is in\n> >>> progress. Document this in the Object class documentation.\n> >>>\n> >>> Signed-off-by: Laurent Pinchart \n> >>> ---\n> >>> src/libcamera/object.cpp | 4 ++++\n> >>> 1 file changed, 4 insertions(+)\n> >>>\n> >>> diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp\n> >>> index 1f787271f782..e76faf48b8ed 100644\n> >>> --- a/src/libcamera/object.cpp\n> >>> +++ b/src/libcamera/object.cpp\n> >>> @@ -40,6 +40,10 @@ LOG_DEFINE_CATEGORY(Object)\n> >>> * implementing easy message passing between threads by inheriting from the\n> >>> * Object class.\n> >>> *\n> >>> + * Deleting an object from a thread other than the one the object is bound to is\n> >>> + * unsafe, unless the caller ensures that the object isn't processing any\n> >>> + * message concurrently.\n> >>> + *\n> >>\n> >> What about codifying this with an assertion or Log(Error) check?\n> >>\n> >> I.e. put something into the destructor which tests the current thread\n> >> against the object bound thread?\n> >>\n> >> I think a runtime check here would be cheap, and serve a better way to\n> >> catch these bugs than simply documenting potential issues.\n> > \n> > I'd do so if it wasn't for the \"unless the caller\" part. It's valid to\n> > destroy objects from other threads if we're careful, and one simple use\n> > case is destroying objects that have been moved to a thread after the\n> > thread finishes.\n> \n> But the message says \n> \"From an object if the object is processing a message\"\n> \n> So doesn't that make the check (psuedo-code):\n> \n> if (pendingMessages_ && this_thread != object->bound_thread())\n> \tLog(Error)\n> \t << \"Destroying object from separate thread with messages\";\n> \n> \n> Or maybe I've misunderstood something?\n\nThe issue isn't pendingMessages_, that's handled by calling\nThread::removeMessages() from Object::~Object(). The issue is the\nobject's message() method being called from Thread::dispatchMessages()\nand the call being in progress when the object is destroyed from another\nthread.\n\n> >>> * Object slots connected to signals will also run in the context of the\n> >>> * object's thread, regardless of whether the signal is emitted in the same or\n> >>> * in another thread.","headers":{"Return-Path":"","Received":["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 A7AE960C33\n\tfor ;\n\tWed, 27 Nov 2019 17:01:20 +0100 (CET)","from pendragon.ideasonboard.com (fs96f9c64d.tkyc509.ap.nuro.jp\n\t[150.249.198.77])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7B3EA325;\n\tWed, 27 Nov 2019 17:01:19 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1574870480;\n\tbh=c4JrM6N27GCG4VX/WHJrHr+Me+n4TFC9WRj8eXxzQlM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=c2NoyQFYlLVBDqdAxS+LJvYwRHAmlyam7Povrk0o6BomliK+B7HGPoL+QAQE17U3e\n\tghS6aCFcxddURH8fPjK/vYi7HgVKCwov6HxvFZulRD8qajXm/FlizGSCf7wWwvTyzh\n\t/NCVWot0/lXBLTmVD9GUuMbcwIzJUxUwc+28WUr8=","Date":"Wed, 27 Nov 2019 18:01:09 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191127160109.GD4730@pendragon.ideasonboard.com>","References":"<20191127084909.10612-1-laurent.pinchart@ideasonboard.com>\n\t<20191127084909.10612-4-laurent.pinchart@ideasonboard.com>\n\t<8921f9df-490d-6569-bf41-e00907d2b146@ideasonboard.com>\n\t<20191127151811.GC4730@pendragon.ideasonboard.com>\n\t<2841d4d1-b58d-cd07-f8b9-e071c027ce58@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<2841d4d1-b58d-cd07-f8b9-e071c027ce58@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: object: Document\n\tdanger of deleting object from other thread","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 27 Nov 2019 16:01:20 -0000"}}]