MadLittleMods / node-usb-detection

List USB devices in system and detect changes on them.
MIT License
371 stars 114 forks source link

Segfault at exit on Linux arm64 in Electron #156

Closed fstanchina closed 2 years ago

fstanchina commented 2 years ago

The usb-detection native module causes a segfault in udev_monitor_receive_device() at exit on Linux arm64. It appears that cbWork() gets called with an invalid monitor object, I guess that on shutdown it's released before the worker stops.

This could explain some sporadic crashes we've seen on Intel. Probably the less powerful hardware causes a race that is rarer on faster machines.

Here's a patch that fixes it, set mon to NULL after releasing (plus some diagnostics). I've actually seen the "Error: udev monitor is null in cbWork()!" message. Probably best would be to do auto tmp = mon; mon = NULL; udev_monitor_unref(tmp); to guarantee that the worker never sees an invalid mon.

diff -ur ../usb-detection_save/src/detection_linux.cpp node_modules/usb-detection/src/detection_linux.cpp
--- ../usb-detection_save/src/detection_linux.cpp   2022-02-07 16:12:26.198991223 +0100
+++ node_modules/usb-detection/src/detection_linux.cpp  2022-02-07 16:23:07.020071344 +0100
@@ -98,7 +98,9 @@
    uv_cond_destroy(&notifyDeviceHandled);

    udev_monitor_unref(mon);
+   mon = NULL;
    udev_unref(udev);
+   udev = NULL;
 }

 void InitDetection() {
@@ -112,6 +114,12 @@

    /* Set up a monitor to monitor devices */
    mon = udev_monitor_new_from_netlink(udev, "udev");
+   if (!mon)
+   {
+       printf("Can't create udev monitor from netlink\n");
+       return;
+   }
+
    udev_monitor_enable_receiving(mon);

    /* Get the file descriptor (fd) for the monitor.
@@ -226,6 +234,12 @@
        if (!ret) continue;
        if (ret < 0) break;

+       if (!mon)
+       {
+           printf("Error: udev monitor is null in cbWork()!\n");
+           return;
+       }
+
        dev = udev_monitor_receive_device(mon);
        if (dev) {
            if(udev_device_get_devtype(dev) && strcmp(udev_device_get_devtype(dev), DEVICE_TYPE_DEVICE) == 0) {
MadLittleMods commented 2 years ago

Hey @fstanchina, it looks like you're fixing the same problem that https://github.com/MadLittleMods/node-usb-detection/pull/138 is trying to address.

And similar to the patch that @kfazz mentioned here https://github.com/MadLittleMods/node-usb-detection/issues/57#issuecomment-877195606

I'm going to close this in favor of those. But an updated PR welcome with the review feedback addressed is welcome!

fstanchina commented 2 years ago

OK, I agree. Sorry, I read thru the existing issues too fast.