[{"id":429,"web_url":"https://patchwork.libcamera.org/comment/429/","msgid":"<20190120180213.ry3htvyzye4mvgaa@uno.localdomain>","date":"2019-01-20T18:02:13","subject":"Re: [libcamera-devel] [PATCH v2 1/4] Documentation: coding_style:\n\tAdd object ownership rules","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\none question: what about objects storing references to other objects?\n\nI'm thinking about the media_device and media_objects.. The\nMediaDevice instanciates media objects, stores references to it, and\npass those references to other objects at creation time (eg. each MediaPad\nhas a reference to its MediaEntity). Should in this case all objects\nbe stored as unique_ptr<>?\n\nEven if you too consider point relevant, don't hold what's in this\nseries for it. We can discuss and add it later, eventually...\n\nThanks\n j\n\nOn Sat, Jan 19, 2019 at 01:26:14AM +0200, Laurent Pinchart wrote:\n> Object ownership is a complex topic that can lead to many issues, from\n> memory leak to crashes. Document the rules that libcamera enforces to\n> make object ownership tracking explicit.\n>\n> This is a first version of the rules and is expected to be expanded as\n> the library is developed.\n>\n> Signed-off-by: Laurent Pinchart \n> Reviewed-by: Niklas Söderlund \n> Reviewed-by: Jacopo Mondi \n> ---\n> Changes since v1:\n>\n> - Clarify documentation\n> - Reference the object ownership rules from the Chromium C++ style guide\n> ---\n> Documentation/coding-style.rst | 84 ++++++++++++++++++++++++++++++++++\n> 1 file changed, 84 insertions(+)\n>\n> diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst\n> index f8d2fdfeda8e..66db3cebe132 100644\n> --- a/Documentation/coding-style.rst\n> +++ b/Documentation/coding-style.rst\n> @@ -81,6 +81,90 @@ C++-11-specific features:\n> overused.\n> * Variadic class and function templates\n>\n> +Object Ownership\n> +~~~~~~~~~~~~~~~~\n> +\n> +libcamera creates and destroys many objects at runtime, for both objects\n> +internal to the library and objects exposed to the user. To guarantee proper\n> +operation without use after free, double free or memory leaks, knowing who owns\n> +each object at any time is crucial. The project has enacted a set of rules to\n> +make object ownership tracking as explicit and fool-proof as possible.\n> +\n> +In the context of this section, the terms object and instance are used\n> +interchangeably and both refer to an instance of a class. The term reference\n> +refers to both C++ references and C++ pointers in their capacity to refer to an\n> +object. Passing a reference means offering a way to a callee to obtain a\n> +reference to an object that the caller has a valid reference to. Borrowing a\n> +reference means using a reference passed by a caller without ownership transfer\n> +based on the assumption that the caller guarantees the validity of the\n> +reference for the duration of the operation that borrows it.\n> +\n> +#. Single Owner Objects\n> +\n> + * By default an object has a single owner at any time.\n> + * Storage of single owner objects varies depending on how the object\n> + ownership will evolve through the lifetime of the object.\n> +\n> + * Objects whose ownership needs to be transferred shall be stored as\n> + std::unique_ptr<> as much as possible to emphasize the single ownership.\n> + * Objects whose owner doesn't change may be embedded in other objects, or\n> + stored as pointer or references. They may be stored as std::unique_ptr<>\n> + for automatic deletion if desired.\n> +\n> + * Ownership is transferred by passing the reference as a std::unique_ptr<>\n> + and using std::move(). After ownership transfer the former owner has no\n> + valid reference to the object anymore and shall not access it without first\n> + obtaining a valid reference.\n> + * Objects may be borrowed by passing an object reference from the owner to\n> + the borrower, providing that\n> +\n> + * the owner guarantees the validity of the reference for the whole duration\n> + of the borrowing, and\n> + * the borrower doesn't access the reference after the end of the borrowing.\n> +\n> + When borrowing from caller to callee for the duration of a function call,\n> + this implies that the callee shall not keep any stored reference after it\n> + returns. These rules apply to the callee and all the functions it calls,\n> + directly or indirectly.\n> +\n> + When the object is stored in a std::unique_ptr<>, borrowing passes a\n> + reference to the object, not to the std::unique_ptr<>, as\n> +\n> + * a 'const &' when the object doesn't need to be modified and may not be\n> + null.\n> + * a pointer when the object may be modified or may be null. Unless\n> + otherwise specified, pointers passed to functions are considered as\n> + borrowed references valid for the duration of the function only.\n> +\n> +#. Shared Objects\n> +\n> + * Objects that may have multiple owners at a given time are called shared\n> + objects. They are reference-counted and live as long as any references to\n> + the object exist.\n> + * Shared objects are created with std::make_shared<> or\n> + std::allocate_shared<> and stored in an std::shared_ptr<>.\n> + * Ownership is shared by creating and passing copies of any valid\n> + std::shared_ptr<>. Ownership is released by destroying the corresponding\n> + std::shared_ptr<>.\n> + * When passed to a function, std::shared_ptr<> are always passed by value,\n> + never by reference. The caller can decide whether to transfer its ownership\n> + of the std::shared_ptr<> with std::move() or retain it. The callee shall\n> + use std::move() if it needs to store the shared pointer.\n> + * Borrowed references to shared objects are passed as references to the\n> + objects themselves, not to the std::shared_ptr<>, with the same rules as\n> + for single owner objects.\n> +\n> +These rules match the `object ownership rules from the Chromium C++ Style Guide`_.\n> +\n> +.. _object ownership rules from the Chromium C++ Style Guide: https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md#object-ownership-and-calling-conventions\n> +\n> +.. attention:: Long term borrowing of single owner objects is allowed. Example\n> + use cases are implementation of the singleton pattern (where the singleton\n> + guarantees the validity of the reference forever), or returning references\n> + to global objects whose lifetime matches the lifetime of the application. As\n> + long term borrowing isn't marked through language constructs, it shall be\n> + documented explicitly in details in the API.\n> +\n>\n> Tools\n> -----\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 relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7A77460C81\n\tfor ;\n\tSun, 20 Jan 2019 19:02:02 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id F394A200006;\n\tSun, 20 Jan 2019 18:02:01 +0000 (UTC)"],"Date":"Sun, 20 Jan 2019 19:02:13 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190120180213.ry3htvyzye4mvgaa@uno.localdomain>","References":"<20190118232617.14631-1-laurent.pinchart@ideasonboard.com>\n\t<20190118232617.14631-2-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"6emtzwmdn266uxfg\"","Content-Disposition":"inline","In-Reply-To":"<20190118232617.14631-2-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 1/4] Documentation: coding_style:\n\tAdd object ownership rules","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sun, 20 Jan 2019 18:02:02 -0000"}},{"id":431,"web_url":"https://patchwork.libcamera.org/comment/431/","msgid":"<20190121004939.GB23038@pendragon.ideasonboard.com>","date":"2019-01-21T00:49:39","subject":"Re: [libcamera-devel] [PATCH v2 1/4] Documentation: coding_style:\n\tAdd object ownership rules","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Sun, Jan 20, 2019 at 07:02:13PM +0100, Jacopo Mondi wrote:\n> Hi Laurent,\n> \n> one question: what about objects storing references to other objects?\n> \n> I'm thinking about the media_device and media_objects.. The\n> MediaDevice instanciates media objects, stores references to it, and\n> pass those references to other objects at creation time (eg. each MediaPad\n> has a reference to its MediaEntity). Should in this case all objects\n> be stored as unique_ptr<>?\n\nShort answer: no. Long answer: no yet :-)\n\nThe current implementation of the media device and related objects\nheavily borrows references. The base assumption is that all the media\nobjects will be valid as long as the media device exist. They can thus\nall reference each other by borrowing references from the media device,\nwith the knowledge that those references will only become invalid when\nthe media device is destroyed, at which time the objects will be\ndestroyed too. The reference borrowing will then stop.\n\nLonger term we'll need to support hot-plugging media devices, and we'll\nhave to decide how to deal with that. The simplest implementation will\nlikely to use std::shared_ptr and keep borrowing references\ninternally between the media objects. Even longer term, when we'll need\nto support dynamic media graphs, this will need to be revisited, and I\ndon't know yet how the API will then be structured.\n\n> Even if you too consider point relevant, don't hold what's in this\n> series for it. We can discuss and add it later, eventually...\n> \n> On Sat, Jan 19, 2019 at 01:26:14AM +0200, Laurent Pinchart wrote:\n> > Object ownership is a complex topic that can lead to many issues, from\n> > memory leak to crashes. Document the rules that libcamera enforces to\n> > make object ownership tracking explicit.\n> >\n> > This is a first version of the rules and is expected to be expanded as\n> > the library is developed.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > Reviewed-by: Niklas Söderlund \n> > Reviewed-by: Jacopo Mondi \n> > ---\n> > Changes since v1:\n> >\n> > - Clarify documentation\n> > - Reference the object ownership rules from the Chromium C++ style guide\n> > ---\n> > Documentation/coding-style.rst | 84 ++++++++++++++++++++++++++++++++++\n> > 1 file changed, 84 insertions(+)\n> >\n> > diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst\n> > index f8d2fdfeda8e..66db3cebe132 100644\n> > --- a/Documentation/coding-style.rst\n> > +++ b/Documentation/coding-style.rst\n> > @@ -81,6 +81,90 @@ C++-11-specific features:\n> > overused.\n> > * Variadic class and function templates\n> >\n> > +Object Ownership\n> > +~~~~~~~~~~~~~~~~\n> > +\n> > +libcamera creates and destroys many objects at runtime, for both objects\n> > +internal to the library and objects exposed to the user. To guarantee proper\n> > +operation without use after free, double free or memory leaks, knowing who owns\n> > +each object at any time is crucial. The project has enacted a set of rules to\n> > +make object ownership tracking as explicit and fool-proof as possible.\n> > +\n> > +In the context of this section, the terms object and instance are used\n> > +interchangeably and both refer to an instance of a class. The term reference\n> > +refers to both C++ references and C++ pointers in their capacity to refer to an\n> > +object. Passing a reference means offering a way to a callee to obtain a\n> > +reference to an object that the caller has a valid reference to. Borrowing a\n> > +reference means using a reference passed by a caller without ownership transfer\n> > +based on the assumption that the caller guarantees the validity of the\n> > +reference for the duration of the operation that borrows it.\n> > +\n> > +#. Single Owner Objects\n> > +\n> > + * By default an object has a single owner at any time.\n> > + * Storage of single owner objects varies depending on how the object\n> > + ownership will evolve through the lifetime of the object.\n> > +\n> > + * Objects whose ownership needs to be transferred shall be stored as\n> > + std::unique_ptr<> as much as possible to emphasize the single ownership.\n> > + * Objects whose owner doesn't change may be embedded in other objects, or\n> > + stored as pointer or references. They may be stored as std::unique_ptr<>\n> > + for automatic deletion if desired.\n> > +\n> > + * Ownership is transferred by passing the reference as a std::unique_ptr<>\n> > + and using std::move(). After ownership transfer the former owner has no\n> > + valid reference to the object anymore and shall not access it without first\n> > + obtaining a valid reference.\n> > + * Objects may be borrowed by passing an object reference from the owner to\n> > + the borrower, providing that\n> > +\n> > + * the owner guarantees the validity of the reference for the whole duration\n> > + of the borrowing, and\n> > + * the borrower doesn't access the reference after the end of the borrowing.\n> > +\n> > + When borrowing from caller to callee for the duration of a function call,\n> > + this implies that the callee shall not keep any stored reference after it\n> > + returns. These rules apply to the callee and all the functions it calls,\n> > + directly or indirectly.\n> > +\n> > + When the object is stored in a std::unique_ptr<>, borrowing passes a\n> > + reference to the object, not to the std::unique_ptr<>, as\n> > +\n> > + * a 'const &' when the object doesn't need to be modified and may not be\n> > + null.\n> > + * a pointer when the object may be modified or may be null. Unless\n> > + otherwise specified, pointers passed to functions are considered as\n> > + borrowed references valid for the duration of the function only.\n> > +\n> > +#. Shared Objects\n> > +\n> > + * Objects that may have multiple owners at a given time are called shared\n> > + objects. They are reference-counted and live as long as any references to\n> > + the object exist.\n> > + * Shared objects are created with std::make_shared<> or\n> > + std::allocate_shared<> and stored in an std::shared_ptr<>.\n> > + * Ownership is shared by creating and passing copies of any valid\n> > + std::shared_ptr<>. Ownership is released by destroying the corresponding\n> > + std::shared_ptr<>.\n> > + * When passed to a function, std::shared_ptr<> are always passed by value,\n> > + never by reference. The caller can decide whether to transfer its ownership\n> > + of the std::shared_ptr<> with std::move() or retain it. The callee shall\n> > + use std::move() if it needs to store the shared pointer.\n> > + * Borrowed references to shared objects are passed as references to the\n> > + objects themselves, not to the std::shared_ptr<>, with the same rules as\n> > + for single owner objects.\n> > +\n> > +These rules match the `object ownership rules from the Chromium C++ Style Guide`_.\n> > +\n> > +.. _object ownership rules from the Chromium C++ Style Guide: https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md#object-ownership-and-calling-conventions\n> > +\n> > +.. attention:: Long term borrowing of single owner objects is allowed. Example\n> > + use cases are implementation of the singleton pattern (where the singleton\n> > + guarantees the validity of the reference forever), or returning references\n> > + to global objects whose lifetime matches the lifetime of the application. As\n> > + long term borrowing isn't marked through language constructs, it shall be\n> > + documented explicitly in details in the API.\n> > +\n> >\n> > Tools\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 D0D3260C6A\n\tfor ;\n\tMon, 21 Jan 2019 01:49:40 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3C667D6C;\n\tMon, 21 Jan 2019 01:49:40 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548031780;\n\tbh=An4JMwBZi5ubTN1gHffuBg6w81T71c2y4C6TsCrhWuw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=K4XYfUOYaUPcMGaRSrFS+sIeCK1i0zPv013tO5MkMyVxKakaU5h0ujYBu8HyM9qKR\n\tISu1ApfG9jhkHhtbjcjupUmluOlb2estUTJQL3CyuSlj107PkHqKx5Gg7tQVByNJ3u\n\tBY4yEJSWxjZtL5pK+x0ir8dq82ngI+I3RrcHHSY4=","Date":"Mon, 21 Jan 2019 02:49:39 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190121004939.GB23038@pendragon.ideasonboard.com>","References":"<20190118232617.14631-1-laurent.pinchart@ideasonboard.com>\n\t<20190118232617.14631-2-laurent.pinchart@ideasonboard.com>\n\t<20190120180213.ry3htvyzye4mvgaa@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190120180213.ry3htvyzye4mvgaa@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 1/4] Documentation: coding_style:\n\tAdd object ownership rules","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 21 Jan 2019 00:49:41 -0000"}}]