[{"id":37304,"web_url":"https://patchwork.libcamera.org/comment/37304/","msgid":"","date":"2025-12-11T12:05:33","subject":"Re: [RFC PATCH v2 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Mon, Dec 08, 2025 at 12:00:35PM +0100, Barnabás Pőcze wrote:\n> It is possible that same device is processed multiple times, leading to\n> multiple `MediaDevice`s being instantiated, mostly likely leading to\n> a fatal error:\n>\n> Trying to register a camera with a duplicated ID xyzw...\n>\n> There is a time window after the `udev_monitor` has been created in `init()`\n> and the first (and only) enumeration done in `enumerate()`. If e.g. a UVC\n> camera is connected in this time frame, then it is possible that it will be\n> processed both by the `udevNotify()` and the initial `enumerate()`, leading\n> to the fatal error. This can be reproduced as follows:\n>\n> 1. $ gdb --args cam -m\n> 2. (gdb) break libcamera::DeviceEnumeratorUdev::enumerate\n> 3. (gdb) run\n> 4. when the breakpoint is hit, connect a usb camera\n> 5. (gdb) continue\n> 6. observe fatal error\n>\n> To address this, keep track of the devnums of all devices reported by\n> udev, and reject devices with already known devnums. This ensures that\n> the same device won't be reported multiple times (assuming that udev\n> reports \"add\" / \"remove\" events in the correct order).\n>\n> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/293\n\nIf only we had a way to notify in an issue that a series targeting it\nis in review...\n\n> Signed-off-by: Barnabás Pőcze \n> Reviewed-by: Kieran Bingham \n> Reviewed-by: Laurent Pinchart \n\nReviewed-by: Jacopo Mondi \n\nThanks!\n\n> ---\n> changes in v2:\n> * address review comments\n>\n> v1: https://patchwork.libcamera.org/patch/25336/\n> ---\n> .../internal/device_enumerator_udev.h | 3 ++\n> src/libcamera/device_enumerator_udev.cpp | 38 ++++++++++++++-----\n> 2 files changed, 32 insertions(+), 9 deletions(-)\n>\n> diff --git a/include/libcamera/internal/device_enumerator_udev.h b/include/libcamera/internal/device_enumerator_udev.h\n> index c2f7154b6b..958e27a2f1 100644\n> --- a/include/libcamera/internal/device_enumerator_udev.h\n> +++ b/include/libcamera/internal/device_enumerator_udev.h\n> @@ -13,6 +13,7 @@\n> #include \n> #include \n> #include \n> +#include \n>\n> #include \n>\n> @@ -59,6 +60,7 @@ private:\n> \tLIBCAMERA_DISABLE_COPY_AND_MOVE(DeviceEnumeratorUdev)\n>\n> \tint addUdevDevice(struct udev_device *dev);\n> +\tvoid removeUdevDevice(struct udev_device *dev);\n> \tint populateMediaDevice(MediaDevice *media, DependencyMap *deps);\n> \tstd::string lookupDeviceNode(dev_t devnum);\n>\n> @@ -70,6 +72,7 @@ private:\n> \tEventNotifier *notifier_;\n>\n> \tstd::set orphans_;\n> +\tstd::unordered_set devices_;\n> \tstd::list pending_;\n> \tstd::map devMap_;\n> };\n> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp\n> index 4e20a3cc0c..3195dd06e2 100644\n> --- a/src/libcamera/device_enumerator_udev.cpp\n> +++ b/src/libcamera/device_enumerator_udev.cpp\n> @@ -76,6 +76,21 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)\n> \tif (!subsystem)\n> \t\treturn -ENODEV;\n>\n> +\t/*\n> +\t * Record that udev reported the given devnum. And reject if it has\n> +\t * already been seen (e.g. device added between udev monitor creation\n> +\t * in `init()` and `enumerate()`). This record is kept even if later\n> +\t * in this function an error is encountered. Only a \"remove\" event\n> +\t * from udev should erase it from `devices_`.\n> +\t */\n> +\tconst dev_t devnum = udev_device_get_devnum(dev);\n> +\tif (devnum == makedev(0, 0))\n> +\t\treturn -ENODEV;\n> +\n> +\tconst auto [it, inserted] = devices_.insert(devnum);\n> +\tif (!inserted)\n> +\t\treturn -EEXIST;\n> +\n> \tif (!strcmp(subsystem, \"media\")) {\n> \t\tstd::unique_ptr media =\n> \t\t\tcreateDevice(udev_device_get_devnode(dev));\n> @@ -111,13 +126,22 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)\n> \t}\n>\n> \tif (!strcmp(subsystem, \"video4linux\")) {\n> -\t\taddV4L2Device(udev_device_get_devnum(dev));\n> +\t\taddV4L2Device(devnum);\n> \t\treturn 0;\n> \t}\n>\n> \treturn -ENODEV;\n> }\n>\n> +void DeviceEnumeratorUdev::removeUdevDevice(struct udev_device *dev)\n> +{\n> +\tconst char *subsystem = udev_device_get_subsystem(dev);\n> +\tif (subsystem && !strcmp(subsystem, \"media\"))\n> +\t\tremoveDevice(udev_device_get_devnode(dev));\n> +\n> +\tdevices_.erase(udev_device_get_devnum(dev));\n> +}\n> +\n> int DeviceEnumeratorUdev::enumerate()\n> {\n> \tstruct udev_enumerate *udev_enum = nullptr;\n> @@ -341,18 +365,14 @@ void DeviceEnumeratorUdev::udevNotify()\n> \t}\n>\n> \tstd::string_view action(udev_device_get_action(dev));\n> -\tstd::string_view deviceNode(udev_device_get_devnode(dev));\n>\n> \tLOG(DeviceEnumerator, Debug)\n> -\t\t<< action << \" device \" << deviceNode;\n> +\t\t<< action << \" device \" << udev_device_get_devnode(dev);\n>\n> -\tif (action == \"add\") {\n> +\tif (action == \"add\")\n> \t\taddUdevDevice(dev);\n> -\t} else if (action == \"remove\") {\n> -\t\tconst char *subsystem = udev_device_get_subsystem(dev);\n> -\t\tif (subsystem && !strcmp(subsystem, \"media\"))\n> -\t\t\tremoveDevice(std::string(deviceNode));\n> -\t}\n> +\telse if (action == \"remove\")\n> +\t\tremoveUdevDevice(dev);\n>\n> \tudev_device_unref(dev);\n> }\n> --\n> 2.52.0","headers":{"Return-Path":"","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 0FBA0BD1F1\n\tfor ;\n\tThu, 11 Dec 2025 12:05:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4923061604;\n\tThu, 11 Dec 2025 13:05:39 +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 B336C609D8\n\tfor ;\n\tThu, 11 Dec 2025 13:05:37 +0100 (CET)","from ideasonboard.com (93-46-82-201.ip106.fastwebnet.it\n\t[93.46.82.201])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CD4B015D2;\n\tThu, 11 Dec 2025 13:05:34 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZmsC104+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1765454734;\n\tbh=smMrMGAIoYSY0TfdWp/L7/5BRPezkOGnG/6Wgea4lJs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZmsC104+KimXrjpmctWY01CDBOUfRBCcayJiYOyuCuqVX9wpwjqZaBpSu80ccVO8c\n\tbrd86ICnqhWQo9dTA0ezciIz8TWYidYEDHofxzT0hUAOE+hTXjxbiq2nFFgwIHAFxp\n\tKK2hdBpNjevUqFdt3V2JnvUoo18I1h+OUgYvPVhI=","Date":"Thu, 11 Dec 2025 13:05:33 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org, \n\tKieran Bingham ,\n\tLaurent Pinchart ","Subject":"Re: [RFC PATCH v2 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices","Message-ID":"","References":"<20251208110035.248881-1-barnabas.pocze@ideasonboard.com>\n\t<20251208110035.248881-3-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20251208110035.248881-3-barnabas.pocze@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]