[{"id":37162,"web_url":"https://patchwork.libcamera.org/comment/37162/","msgid":"<176468459101.2003224.15714270685458236848@ping.linuxembedded.co.uk>","date":"2025-12-02T14:09:51","subject":"Re: [RFC PATCH v1 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-12-02 13:58:17)\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> This can most easily happen as a result of multiple `enumerate()` calls,\n> however, currently `enumerate()` is only ever called once, so that won't\n> trigger the issue.\n> \n> Nonetheless, it is still possible to trigger this issue due to the fact\n> that there is a time window after the `udev_monitor` has been created in\n> `init()` and the first (and only) enumeration done in `enumerate()`. If\n> e.g. a UVC camera is connected in this time frame, then it is possible\n> that it will be processed both by the `udevNotify()` and the initial\n> `enumerate()`, leading to the fatal error. This can be reproduced as\n> 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\nThis seems like such a small race window - it's surprising if that's\nwhat caused the reported bug. But if it's a window it can happen right\n...\n\nBut the patch looks reasonable to me:\n\nReviewed-by: Kieran Bingham \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> any number of `enumerate()` calls and hotplug events will work correctly\n> (assuming that udev reports \"add\" / \"remove\" events correctly).\n> \n> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/293\n> Signed-off-by: Barnabás Pőcze \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..3de7c37104 100644\n> --- a/include/libcamera/internal/device_enumerator_udev.h\n> +++ b/include/libcamera/internal/device_enumerator_udev.h\n> @@ -11,6 +11,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> \n> @@ -59,6 +60,7 @@ private:\n> LIBCAMERA_DISABLE_COPY_AND_MOVE(DeviceEnumeratorUdev)\n> \n> int addUdevDevice(struct udev_device *dev);\n> + void removeUdevDevice(struct udev_device *dev);\n> int populateMediaDevice(MediaDevice *media, DependencyMap *deps);\n> std::string lookupDeviceNode(dev_t devnum);\n> \n> @@ -70,6 +72,7 @@ private:\n> EventNotifier *notifier_;\n> \n> std::set orphans_;\n> + std::unordered_set devices_;\n> std::list pending_;\n> std::map devMap_;\n> };\n> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp\n> index 4e20a3cc0c..406e59b360 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> if (!subsystem)\n> return -ENODEV;\n> \n> + /*\n> + * Record that udev reported the given devnum. And reject if it has already\n> + * been seen (e.g. multiple `enumerate()` calls, device added between udev\n> + * monitor creation in `init()` and `enumerate()`). This record is kept even\n> + * if later in this function an error is encountered. Only a \"remove\" event from\n> + * udev should erase it from `devices_`.\n> + */\n> + const dev_t devnum = udev_device_get_devnum(dev);\n> + if (devnum == makedev(0, 0))\n> + return -ENODEV;\n> +\n> + const auto [it, inserted] = devices_.insert(devnum);\n> + if (!inserted)\n> + return -EEXIST;\n> +\n> if (!strcmp(subsystem, \"media\")) {\n> std::unique_ptr media =\n> createDevice(udev_device_get_devnode(dev));\n> @@ -111,13 +126,22 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)\n> }\n> \n> if (!strcmp(subsystem, \"video4linux\")) {\n> - addV4L2Device(udev_device_get_devnum(dev));\n> + addV4L2Device(devnum);\n> return 0;\n> }\n> \n> return -ENODEV;\n> }\n> \n> +void DeviceEnumeratorUdev::removeUdevDevice(struct udev_device *dev)\n> +{\n> + const char *subsystem = udev_device_get_subsystem(dev);\n> + if (subsystem && !strcmp(subsystem, \"media\"))\n> + removeDevice(udev_device_get_devnode(dev));\n> +\n> + devices_.erase(udev_device_get_devnum(dev));\n> +}\n> +\n> int DeviceEnumeratorUdev::enumerate()\n> {\n> struct udev_enumerate *udev_enum = nullptr;\n> @@ -341,18 +365,14 @@ void DeviceEnumeratorUdev::udevNotify()\n> }\n> \n> std::string_view action(udev_device_get_action(dev));\n> - std::string_view deviceNode(udev_device_get_devnode(dev));\n> \n> LOG(DeviceEnumerator, Debug)\n> - << action << \" device \" << deviceNode;\n> + << action << \" device \" << udev_device_get_devnode(dev);\n> \n> - if (action == \"add\") {\n> + if (action == \"add\")\n> addUdevDevice(dev);\n> - } else if (action == \"remove\") {\n> - const char *subsystem = udev_device_get_subsystem(dev);\n> - if (subsystem && !strcmp(subsystem, \"media\"))\n> - removeDevice(std::string(deviceNode));\n> - }\n> + else if (action == \"remove\")\n> + removeUdevDevice(dev);\n> \n> udev_device_unref(dev);\n> }\n> -- \n> 2.52.0\n>","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 1E3D2C3257\n\tfor ;\n\tTue, 2 Dec 2025 14:09:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 06A6560C8C;\n\tTue, 2 Dec 2025 15:09:56 +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 F33476096B\n\tfor ;\n\tTue, 2 Dec 2025 15:09:53 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B4AE843;\n\tTue, 2 Dec 2025 15:07:39 +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=\"bQh8BAs2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764684459;\n\tbh=ygwMj4dna4liAmHZHsS1Y4YbtuADA+hm5sSFehYPOAQ=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=bQh8BAs2FojS1quuqxDFkWuNzea/UJBML+F8hdNMNS6vgO9Yaik6rdO7F6ZK1R2R4\n\tPc4kzAi4fdEA5MZg7JAE6/R70YEKjZVyq/85+m0Ru3zYSndQQJcQUNsmjdIRRpoWzO\n\tWVWBK5Z6qxMVivLwLon7k89iLtoB53aSelKO5nqA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251202135817.1607250-3-barnabas.pocze@ideasonboard.com>","References":"<20251202135817.1607250-1-barnabas.pocze@ideasonboard.com>\n\t<20251202135817.1607250-3-barnabas.pocze@ideasonboard.com>","Subject":"Re: [RFC PATCH v1 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 02 Dec 2025 14:09:51 +0000","Message-ID":"<176468459101.2003224.15714270685458236848@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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\" "}},{"id":37163,"web_url":"https://patchwork.libcamera.org/comment/37163/","msgid":"<041c614f-b59a-4fb1-baff-a4447892df77@ideasonboard.com>","date":"2025-12-02T14:15:49","subject":"Re: [RFC PATCH v1 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 12. 02. 15:09 keltezéssel, Kieran Bingham írta:\n> Quoting Barnabás Pőcze (2025-12-02 13:58:17)\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>> This can most easily happen as a result of multiple `enumerate()` calls,\n>> however, currently `enumerate()` is only ever called once, so that won't\n>> trigger the issue.\n>>\n>> Nonetheless, it is still possible to trigger this issue due to the fact\n>> that there is a time window after the `udev_monitor` has been created in\n>> `init()` and the first (and only) enumeration done in `enumerate()`. If\n>> e.g. a UVC camera is connected in this time frame, then it is possible\n>> that it will be processed both by the `udevNotify()` and the initial\n>> `enumerate()`, leading to the fatal error. This can be reproduced as\n>> 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> \n> This seems like such a small race window - it's surprising if that's\n> what caused the reported bug. But if it's a window it can happen right\n\nThe reporter said\n\n libcamera in wireplumber crashed when I updated my system and Wireplumber restarted, here are logs:\n\nSo I imagine, due to the system update, there was some (a lot?) udev activity in\nparallel with wireplumber restarting, which made the bug visible.\n\n\n> ...\n> \n> But the patch looks reasonable to me:\n> \n> Reviewed-by: Kieran Bingham \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>> any number of `enumerate()` calls and hotplug events will work correctly\n>> (assuming that udev reports \"add\" / \"remove\" events correctly).\n>>\n>> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/293\n>> Signed-off-by: Barnabás Pőcze \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..3de7c37104 100644\n>> --- a/include/libcamera/internal/device_enumerator_udev.h\n>> +++ b/include/libcamera/internal/device_enumerator_udev.h\n>> @@ -11,6 +11,7 @@\n>> #include \n>> #include \n>> #include \n>> +#include \n>> #include \n>> #include \n>> \n>> @@ -59,6 +60,7 @@ private:\n>> LIBCAMERA_DISABLE_COPY_AND_MOVE(DeviceEnumeratorUdev)\n>> \n>> int addUdevDevice(struct udev_device *dev);\n>> + void removeUdevDevice(struct udev_device *dev);\n>> int populateMediaDevice(MediaDevice *media, DependencyMap *deps);\n>> std::string lookupDeviceNode(dev_t devnum);\n>> \n>> @@ -70,6 +72,7 @@ private:\n>> EventNotifier *notifier_;\n>> \n>> std::set orphans_;\n>> + std::unordered_set devices_;\n>> std::list pending_;\n>> std::map devMap_;\n>> };\n>> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp\n>> index 4e20a3cc0c..406e59b360 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>> if (!subsystem)\n>> return -ENODEV;\n>> \n>> + /*\n>> + * Record that udev reported the given devnum. And reject if it has already\n>> + * been seen (e.g. multiple `enumerate()` calls, device added between udev\n>> + * monitor creation in `init()` and `enumerate()`). This record is kept even\n>> + * if later in this function an error is encountered. Only a \"remove\" event from\n>> + * udev should erase it from `devices_`.\n>> + */\n>> + const dev_t devnum = udev_device_get_devnum(dev);\n>> + if (devnum == makedev(0, 0))\n>> + return -ENODEV;\n>> +\n>> + const auto [it, inserted] = devices_.insert(devnum);\n>> + if (!inserted)\n>> + return -EEXIST;\n>> +\n>> if (!strcmp(subsystem, \"media\")) {\n>> std::unique_ptr media =\n>> createDevice(udev_device_get_devnode(dev));\n>> @@ -111,13 +126,22 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)\n>> }\n>> \n>> if (!strcmp(subsystem, \"video4linux\")) {\n>> - addV4L2Device(udev_device_get_devnum(dev));\n>> + addV4L2Device(devnum);\n>> return 0;\n>> }\n>> \n>> return -ENODEV;\n>> }\n>> \n>> +void DeviceEnumeratorUdev::removeUdevDevice(struct udev_device *dev)\n>> +{\n>> + const char *subsystem = udev_device_get_subsystem(dev);\n>> + if (subsystem && !strcmp(subsystem, \"media\"))\n>> + removeDevice(udev_device_get_devnode(dev));\n>> +\n>> + devices_.erase(udev_device_get_devnum(dev));\n>> +}\n>> +\n>> int DeviceEnumeratorUdev::enumerate()\n>> {\n>> struct udev_enumerate *udev_enum = nullptr;\n>> @@ -341,18 +365,14 @@ void DeviceEnumeratorUdev::udevNotify()\n>> }\n>> \n>> std::string_view action(udev_device_get_action(dev));\n>> - std::string_view deviceNode(udev_device_get_devnode(dev));\n>> \n>> LOG(DeviceEnumerator, Debug)\n>> - << action << \" device \" << deviceNode;\n>> + << action << \" device \" << udev_device_get_devnode(dev);\n>> \n>> - if (action == \"add\") {\n>> + if (action == \"add\")\n>> addUdevDevice(dev);\n>> - } else if (action == \"remove\") {\n>> - const char *subsystem = udev_device_get_subsystem(dev);\n>> - if (subsystem && !strcmp(subsystem, \"media\"))\n>> - removeDevice(std::string(deviceNode));\n>> - }\n>> + else if (action == \"remove\")\n>> + removeUdevDevice(dev);\n>> \n>> udev_device_unref(dev);\n>> }\n>> -- \n>> 2.52.0\n>>","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 8E4FEBD80A\n\tfor ;\n\tTue, 2 Dec 2025 14:15:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9CDF260C8C;\n\tTue, 2 Dec 2025 15:15:55 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6C6806096B\n\tfor ;\n\tTue, 2 Dec 2025 15:15:53 +0100 (CET)","from [192.168.33.23] (185.182.214.104.nat.pool.zt.hu\n\t[185.182.214.104])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 03A7B110;\n\tTue, 2 Dec 2025 15:13:38 +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=\"iDy1dLZS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764684819;\n\tbh=p8ERGWpw97fdmoGOEK+CDdI/BRu0YELF6DILFX9xRE4=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=iDy1dLZSilVJdniW4aLwHsMoa7sDdKb4u9ZKR3i0th8G2hrkmQwQvHAe7qp2aq0rn\n\tgScDGWBgxhyOY7rYALCgdYD4qRRZkqfEpJM/lj/Omt0PD8JwnXulpuvp3VpaaAKMFs\n\t2PtJnGCasTZuBdkft/zc9b+l6VcmL4TZQPCMWVUA=","Message-ID":"<041c614f-b59a-4fb1-baff-a4447892df77@ideasonboard.com>","Date":"Tue, 2 Dec 2025 15:15:49 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v1 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251202135817.1607250-1-barnabas.pocze@ideasonboard.com>\n\t<20251202135817.1607250-3-barnabas.pocze@ideasonboard.com>\n\t<176468459101.2003224.15714270685458236848@ping.linuxembedded.co.uk>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<176468459101.2003224.15714270685458236848@ping.linuxembedded.co.uk>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}},{"id":37165,"web_url":"https://patchwork.libcamera.org/comment/37165/","msgid":"<20251202154302.GC8219@pendragon.ideasonboard.com>","date":"2025-12-02T15:43:02","subject":"Re: [RFC PATCH v1 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnabás,\n\nThank you for addressing this long-standing issue.\n\nOn Tue, Dec 02, 2025 at 02:58:17PM +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> This can most easily happen as a result of multiple `enumerate()` calls,\n> however, currently `enumerate()` is only ever called once, so that won't\n> trigger the issue.\n> \n> Nonetheless, it is still possible to trigger this issue due to the fact\n> that there is a time window after the `udev_monitor` has been created in\n> `init()` and the first (and only) enumeration done in `enumerate()`. If\n> e.g. a UVC camera is connected in this time frame, then it is possible\n> that it will be processed both by the `udevNotify()` and the initial\n> `enumerate()`, leading to the fatal error. This can be reproduced as\n> 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> any number of `enumerate()` calls and hotplug events will work correctly\n> (assuming that udev reports \"add\" / \"remove\" events correctly).\n\nA race between initial enumerate and hotplug events is unavoidable with\nour code structure. As this is an issue that I would expect to affect\nlots of applications using udev, I wonder if there's a recommended way\nin the udev API to handle this. Are all applications expected to filter\nout duplicates manually ?\n\n> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/293\n> Signed-off-by: Barnabás Pőcze \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..3de7c37104 100644\n> --- a/include/libcamera/internal/device_enumerator_udev.h\n> +++ b/include/libcamera/internal/device_enumerator_udev.h\n> @@ -11,6 +11,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \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..406e59b360 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 already\n> +\t * been seen (e.g. multiple `enumerate()` calls, device added between udev\n> +\t * monitor creation in `init()` and `enumerate()`). This record is kept even\n> +\t * if later in this function an error is encountered. Only a \"remove\" event from\n> +\t * 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> }","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 DCDC9BD80A\n\tfor ;\n\tTue, 2 Dec 2025 15:43:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C875160D2F;\n\tTue, 2 Dec 2025 16:43:26 +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 0422160C8A\n\tfor ;\n\tTue, 2 Dec 2025 16:43:25 +0100 (CET)","from pendragon.ideasonboard.com\n\t(p9411226-ipngn12302marunouchi.tokyo.ocn.ne.jp [153.160.235.226])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id E91AC161;\n\tTue, 2 Dec 2025 16:41:09 +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=\"qjVP2GYn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764690070;\n\tbh=NTvLD+5R4cMkEg+ma8xbIG6RCyqN2+hSf+24IoPoers=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qjVP2GYnu9prqaO6Y+8rJxm1qlJEEIkhSrNZYSYJYpkznpHEkjH08t721PXkkZTfT\n\t6Xb908N+HmDo5xGAZcY4LFxgr4aFh+SZNF28PXGDZHnr7p10cDI8tz6JyU99/EL1HM\n\th9mLwLsiHA8JMYSf+BVinOfjOBSSbFhhV8JgU/2E=","Date":"Wed, 3 Dec 2025 00:43:02 +0900","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v1 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices","Message-ID":"<20251202154302.GC8219@pendragon.ideasonboard.com>","References":"<20251202135817.1607250-1-barnabas.pocze@ideasonboard.com>\n\t<20251202135817.1607250-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":"<20251202135817.1607250-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\" "}},{"id":37169,"web_url":"https://patchwork.libcamera.org/comment/37169/","msgid":"","date":"2025-12-02T17:09:45","subject":"Re: [RFC PATCH v1 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 12. 02. 16:43 keltezéssel, Laurent Pinchart írta:\n> Hi Barnabás,\n> \n> Thank you for addressing this long-standing issue.\n> \n> On Tue, Dec 02, 2025 at 02:58:17PM +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>> This can most easily happen as a result of multiple `enumerate()` calls,\n>> however, currently `enumerate()` is only ever called once, so that won't\n>> trigger the issue.\n>>\n>> Nonetheless, it is still possible to trigger this issue due to the fact\n>> that there is a time window after the `udev_monitor` has been created in\n>> `init()` and the first (and only) enumeration done in `enumerate()`. If\n>> e.g. a UVC camera is connected in this time frame, then it is possible\n>> that it will be processed both by the `udevNotify()` and the initial\n>> `enumerate()`, leading to the fatal error. This can be reproduced as\n>> 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>> any number of `enumerate()` calls and hotplug events will work correctly\n>> (assuming that udev reports \"add\" / \"remove\" events correctly).\n> \n> A race between initial enumerate and hotplug events is unavoidable with\n> our code structure. As this is an issue that I would expect to affect\n> lots of applications using udev, I wonder if there's a recommended way\n> in the udev API to handle this. Are all applications expected to filter\n> out duplicates manually ?\n\nMy conclusion is \"yes(?)\". The udev enumerator and monitor are largely\nseparate things, they do refer to a common `struct udev` context, but\nat least in the systemd implementation, that only stores a \"user data\"\n`void *`. So I don't think we can expect udev to mediate this. I also\ncannot see any way to \"connect\" the enumerator and the monitor.\n\nI have looked at multiple users on debian code search, and while it wasn't\ntoo conclusive, I have seen deduplication code for sure. E.g. libinput\nexplicitly mentions this very race condition:\n\n\t/* There is a race at startup: a device added between setting\n\t * up the udev monitor and enumerating all current devices may show\n\t * up in both lists. Filter those out.\n\t */\n\tif (filter_duplicates(seat, udev_device))\n\t\treturn 0;\n\n -- https://sources.debian.org/src/libinput/1.30.0-1/src/udev-seat.c?hl=57#L103\n\n\nRegards,\nBarnabás Pőcze\n\n> \n>> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/293\n>> Signed-off-by: Barnabás Pőcze \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..3de7c37104 100644\n>> --- a/include/libcamera/internal/device_enumerator_udev.h\n>> +++ b/include/libcamera/internal/device_enumerator_udev.h\n>> @@ -11,6 +11,7 @@\n>> #include \n>> #include \n>> #include \n>> +#include \n>> #include \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..406e59b360 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 already\n>> +\t * been seen (e.g. multiple `enumerate()` calls, device added between udev\n>> +\t * monitor creation in `init()` and `enumerate()`). This record is kept even\n>> +\t * if later in this function an error is encountered. Only a \"remove\" event from\n>> +\t * 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>","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 A3640C3257\n\tfor ;\n\tTue, 2 Dec 2025 17:09:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C179760D28;\n\tTue, 2 Dec 2025 18:09:50 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 01E1A60C8A\n\tfor ;\n\tTue, 2 Dec 2025 18:09:48 +0100 (CET)","from [192.168.33.23] (185.182.214.104.nat.pool.zt.hu\n\t[185.182.214.104])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 97836FE;\n\tTue, 2 Dec 2025 18:07: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=\"JB+BY3Gm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764695254;\n\tbh=XXbrAk/yVYboXgpyHQCw+Ea00cyxHuJC4igdWZek5Ec=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=JB+BY3Gmn03CSmm0gpnEo8LdpGB9kNplUe52Hlfe/EZczLvtCiArik9qC0cS6yzpn\n\tIxswhqtdMqjHroxXoSJcrp470mrqWUUgY2BOQGuRVg8oTjJPJjxgPf4HXhkuLxSJm2\n\tVfHRFo/blBwTQ3R4jGV3IrfiHaHLkn28/m3A9T+0=","Message-ID":"","Date":"Tue, 2 Dec 2025 18:09:45 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v1 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20251202135817.1607250-1-barnabas.pocze@ideasonboard.com>\n\t<20251202135817.1607250-3-barnabas.pocze@ideasonboard.com>\n\t<20251202154302.GC8219@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251202154302.GC8219@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}},{"id":37175,"web_url":"https://patchwork.libcamera.org/comment/37175/","msgid":"<20251203021512.GH8219@pendragon.ideasonboard.com>","date":"2025-12-03T02:15:12","subject":"Re: [RFC PATCH v1 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Dec 02, 2025 at 06:09:45PM +0100, Barnabás Pőcze wrote:\n> 2025. 12. 02. 16:43 keltezéssel, Laurent Pinchart írta:\n> > On Tue, Dec 02, 2025 at 02:58:17PM +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> >> This can most easily happen as a result of multiple `enumerate()` calls,\n> >> however, currently `enumerate()` is only ever called once, so that won't\n> >> trigger the issue.\n> >>\n> >> Nonetheless, it is still possible to trigger this issue due to the fact\n> >> that there is a time window after the `udev_monitor` has been created in\n> >> `init()` and the first (and only) enumeration done in `enumerate()`. If\n> >> e.g. a UVC camera is connected in this time frame, then it is possible\n> >> that it will be processed both by the `udevNotify()` and the initial\n> >> `enumerate()`, leading to the fatal error. This can be reproduced as\n> >> 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> >> any number of `enumerate()` calls and hotplug events will work correctly\n> >> (assuming that udev reports \"add\" / \"remove\" events correctly).\n> > \n> > A race between initial enumerate and hotplug events is unavoidable with\n> > our code structure. As this is an issue that I would expect to affect\n> > lots of applications using udev, I wonder if there's a recommended way\n> > in the udev API to handle this. Are all applications expected to filter\n> > out duplicates manually ?\n> \n> My conclusion is \"yes(?)\". The udev enumerator and monitor are largely\n> separate things, they do refer to a common `struct udev` context, but\n> at least in the systemd implementation, that only stores a \"user data\"\n> `void *`. So I don't think we can expect udev to mediate this. I also\n> cannot see any way to \"connect\" the enumerator and the monitor.\n> \n> I have looked at multiple users on debian code search, and while it wasn't\n> too conclusive, I have seen deduplication code for sure. E.g. libinput\n> explicitly mentions this very race condition:\n> \n> \t/* There is a race at startup: a device added between setting\n> \t * up the udev monitor and enumerating all current devices may show\n> \t * up in both lists. Filter those out.\n> \t */\n> \tif (filter_duplicates(seat, udev_device))\n> \t\treturn 0;\n> \n> -- https://sources.debian.org/src/libinput/1.30.0-1/src/udev-seat.c?hl=57#L103\n\nThank you for the information. It would have made sense for udev to\nexpose some sort of API to handle this, instead of causing dozens of\nusers to implement slightly different bugs :-)\n\n> >> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/293\n> >> Signed-off-by: Barnabás Pőcze \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..3de7c37104 100644\n> >> --- a/include/libcamera/internal/device_enumerator_udev.h\n> >> +++ b/include/libcamera/internal/device_enumerator_udev.h\n> >> @@ -11,6 +11,7 @@\n> >> #include \n> >> #include \n> >> #include \n> >> +#include \n\nWrong order (pun intended).\n\n> >> #include \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..406e59b360 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 already\n> >> +\t * been seen (e.g. multiple `enumerate()` calls, device added between udev\n\nenumerate() is not meant to be called multiple times, so I wouldn't have\nmentioned guarding against that as a goal here (or in the commit\nmessage). If you want, you can add a sentence to the function's\ndocumentation to indicate the function can be called once only.\n\n> >> +\t * monitor creation in `init()` and `enumerate()`). This record is kept even\n> >> +\t * if later in this function an error is encountered. Only a \"remove\" event from\n> >> +\t * udev should erase it from `devices_`.\n\nPlease reflow comments to 80 columns.\n\n> >> +\t */\n> >> +\tconst dev_t devnum = udev_device_get_devnum(dev);\n\nI wonder if there could be other types of race conditions that would\ncause different devices to be seen with the same devnum (when a device\nis removed and then re-added for instance). I can't think of any though.\n\nReviewed-by: Laurent Pinchart \n\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> >> }","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 EE915BD80A\n\tfor ;\n\tWed, 3 Dec 2025 02:15:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9DAB960E3E;\n\tWed, 3 Dec 2025 03:15:35 +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 25F9C606D5\n\tfor ;\n\tWed, 3 Dec 2025 03:15:33 +0100 (CET)","from pendragon.ideasonboard.com (p100198.f.east.v6connect.net\n\t[221.113.100.198])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id EEB821691; \n\tWed, 3 Dec 2025 03:13:16 +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=\"AOvgvsR6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764727997;\n\tbh=MN5FWAL38rimIzFug3bzqovx67oEqXgjS8XxW3o9lZk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=AOvgvsR6N9VDxn9CfP7YHi0G+8TNbNHbnr1yV0PdYq8uXs5XRJCvYom5qMrclI/Ld\n\tiKXwQf2Akjm/gbE1/8mbFws7DB/nMbEkpts9qLzt1oFedFnR27odLp6lSEfKb8K1Me\n\tzw6O63NOnzYD1KUxxlgfZDhM4J8tXr3F6kFHHnQo=","Date":"Wed, 3 Dec 2025 11:15:12 +0900","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v1 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices","Message-ID":"<20251203021512.GH8219@pendragon.ideasonboard.com>","References":"<20251202135817.1607250-1-barnabas.pocze@ideasonboard.com>\n\t<20251202135817.1607250-3-barnabas.pocze@ideasonboard.com>\n\t<20251202154302.GC8219@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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\" "}},{"id":37226,"web_url":"https://patchwork.libcamera.org/comment/37226/","msgid":"<19fb38b1-0c08-4479-8de7-591a8248e5b2@ideasonboard.com>","date":"2025-12-08T10:54:31","subject":"Re: [RFC PATCH v1 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 12. 03. 3:15 keltezéssel, Laurent Pinchart írta:\n> On Tue, Dec 02, 2025 at 06:09:45PM +0100, Barnabás Pőcze wrote:\n>> 2025. 12. 02. 16:43 keltezéssel, Laurent Pinchart írta:\n>>> On Tue, Dec 02, 2025 at 02:58:17PM +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>>>> This can most easily happen as a result of multiple `enumerate()` calls,\n>>>> however, currently `enumerate()` is only ever called once, so that won't\n>>>> trigger the issue.\n>>>>\n>>>> Nonetheless, it is still possible to trigger this issue due to the fact\n>>>> that there is a time window after the `udev_monitor` has been created in\n>>>> `init()` and the first (and only) enumeration done in `enumerate()`. If\n>>>> e.g. a UVC camera is connected in this time frame, then it is possible\n>>>> that it will be processed both by the `udevNotify()` and the initial\n>>>> `enumerate()`, leading to the fatal error. This can be reproduced as\n>>>> 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>>>> any number of `enumerate()` calls and hotplug events will work correctly\n>>>> (assuming that udev reports \"add\" / \"remove\" events correctly).\n>>>\n>>> A race between initial enumerate and hotplug events is unavoidable with\n>>> our code structure. As this is an issue that I would expect to affect\n>>> lots of applications using udev, I wonder if there's a recommended way\n>>> in the udev API to handle this. Are all applications expected to filter\n>>> out duplicates manually ?\n>>\n>> My conclusion is \"yes(?)\". The udev enumerator and monitor are largely\n>> separate things, they do refer to a common `struct udev` context, but\n>> at least in the systemd implementation, that only stores a \"user data\"\n>> `void *`. So I don't think we can expect udev to mediate this. I also\n>> cannot see any way to \"connect\" the enumerator and the monitor.\n>>\n>> I have looked at multiple users on debian code search, and while it wasn't\n>> too conclusive, I have seen deduplication code for sure. E.g. libinput\n>> explicitly mentions this very race condition:\n>>\n>> \t/* There is a race at startup: a device added between setting\n>> \t * up the udev monitor and enumerating all current devices may show\n>> \t * up in both lists. Filter those out.\n>> \t */\n>> \tif (filter_duplicates(seat, udev_device))\n>> \t\treturn 0;\n>>\n>> -- https://sources.debian.org/src/libinput/1.30.0-1/src/udev-seat.c?hl=57#L103\n> \n> Thank you for the information. It would have made sense for udev to\n> expose some sort of API to handle this, instead of causing dozens of\n> users to implement slightly different bugs :-)\n> \n>>>> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/293\n>>>> Signed-off-by: Barnabás Pőcze \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..3de7c37104 100644\n>>>> --- a/include/libcamera/internal/device_enumerator_udev.h\n>>>> +++ b/include/libcamera/internal/device_enumerator_udev.h\n>>>> @@ -11,6 +11,7 @@\n>>>> #include \n>>>> #include \n>>>> #include \n>>>> +#include \n> \n> Wrong order (pun intended).\n> \n>>>> #include \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..406e59b360 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 already\n>>>> +\t * been seen (e.g. multiple `enumerate()` calls, device added between udev\n> \n> enumerate() is not meant to be called multiple times, so I wouldn't have\n> mentioned guarding against that as a goal here (or in the commit\n> message). If you want, you can add a sentence to the function's\n> documentation to indicate the function can be called once only.\n> \n>>>> +\t * monitor creation in `init()` and `enumerate()`). This record is kept even\n>>>> +\t * if later in this function an error is encountered. Only a \"remove\" event from\n>>>> +\t * udev should erase it from `devices_`.\n> \n> Please reflow comments to 80 columns.\n> \n>>>> +\t */\n>>>> +\tconst dev_t devnum = udev_device_get_devnum(dev);\n> \n> I wonder if there could be other types of race conditions that would\n> cause different devices to be seen with the same devnum (when a device\n> is removed and then re-added for instance). I can't think of any though.\n\nI think this part already depends on the unique-ness of the devnum,\nsee the `orphans_` set. So as long as that holds, and udev reports\nadd/remove events in the correct order, I think it should be fine.\n\n\n> \n> Reviewed-by: Laurent Pinchart \n> \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> --\n> Regards,\n> \n> Laurent Pinchart","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 5CC23C3257\n\tfor ;\n\tMon, 8 Dec 2025 10:54:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4571D613FF;\n\tMon, 8 Dec 2025 11:54:41 +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 D08AF60C8A\n\tfor ;\n\tMon, 8 Dec 2025 11:54:39 +0100 (CET)","from [192.168.33.35] (185.221.142.68.nat.pool.zt.hu\n\t[185.221.142.68])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 519A916A;\n\tMon, 8 Dec 2025 11:52:20 +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=\"SX20kymj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1765191140;\n\tbh=yo+VaGijIh5S80QonLCz3ehNcgzJ5Jo72pHkLq4iUhc=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=SX20kymjKzNhvHEr3ZKDI8eGNW6B8m4RrlzBv0N2gP2p6C6NVI/OQZKcRPpD2UdNR\n\tTkMpa9dENwbloluxxkZOyWvj5yFJkmRlXm/0xrMDm7UvB4DEb/5pcWQS8/X+Q2hhRI\n\tHvp2sAiXLzg1FSiCbcs4wDj60IIUq1xc6YqYijow=","Message-ID":"<19fb38b1-0c08-4479-8de7-591a8248e5b2@ideasonboard.com>","Date":"Mon, 8 Dec 2025 11:54:31 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v1 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices","To":"Laurent Pinchart , =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?= ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20251202135817.1607250-1-barnabas.pocze@ideasonboard.com>\n\t<20251202135817.1607250-3-barnabas.pocze@ideasonboard.com>\n\t<20251202154302.GC8219@pendragon.ideasonboard.com>\n\t\n\t<-t441_6rqhJYlFfMpfpVAhsQlO21Ih39YoHOTAiH7n7Sf1TW7_ZEHitFJVXIJnt1jKlMd9QkTt7K8ChIImc94w==@protonmail.internalid>\n\t<20251203021512.GH8219@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251203021512.GH8219@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}}]