From 174f5436a163ffc9c457988faac32d8107782938 Mon Sep 17 00:00:00 2001 From: Andrei Sarakeev Date: Thu, 9 Nov 2017 11:09:33 -0600 Subject: [PATCH] Fix 100% CPU and switch to non-blocking IO Port #21 on top of the latest `master` > - Migration from `pthread` to `libuv`. > - Fix 100% CPU usage (using `poll()`): > source > > ``` c > /** > * udev_monitor_receive_device: > * @udev_monitor: udev monitor > ... > * The monitor socket is by default set to NONBLOCK. A variant of poll() on > * the file descriptor returned by udev_monitor_get_fd() should to be used to > * wake up when new devices arrive, or alternatively the file descriptor > * switched into blocking mode. > ... > **/ > ``` > - Out of the loop when receives SIGINT or SIGTERM signals for accurate completion of the program > - Fix some ome memory leaks > > ``` c > udev_monitor_unref(mon); > udev_unref(udev); > ``` > - For local functions and variables using the keyword `static` > > https://github.com/MadLittleMods/node-usb-detection/pull/21#issue-120115183 --- src/detection_linux.cpp | 180 ++++++++++++++++++++-------------------- 1 file changed, 89 insertions(+), 91 deletions(-) diff --git a/src/detection_linux.cpp b/src/detection_linux.cpp index aaa618f..c2b0230 100644 --- a/src/detection_linux.cpp +++ b/src/detection_linux.cpp @@ -1,5 +1,5 @@ #include -#include +#include #include "detection.h" #include "deviceList.h" @@ -30,72 +30,51 @@ using namespace std; /********************************** * Local Variables **********************************/ -ListResultItem_t* currentItem; -bool isAdded; +static ListResultItem_t* currentItem; +static bool isAdded; -struct udev *udev; -struct udev_enumerate *enumerate; -struct udev_list_entry *devices, *dev_list_entry; -struct udev_device *dev; +static udev *udev; +static udev_enumerate *enumerate; +static udev_list_entry *devices, *dev_list_entry; +static udev_device *dev; -struct udev_monitor *mon; -int fd; +static udev_monitor *mon; +static int fd; -pthread_t thread; -pthread_mutex_t notify_mutex; -pthread_cond_t notifyNewDevice; -pthread_cond_t notifyDeviceHandled; +static uv_work_t work_req; +static uv_signal_t term_signal; +static uv_signal_t int_signal; -bool newDeviceAvailable = false; -bool deviceHandled = true; +static uv_async_t async_handler; +static uv_mutex_t notify_mutex; +static uv_cond_t notifyDeviceHandled; + +static bool deviceHandled = true; + +static bool isRunning = false; +static bool quit = false; -bool isRunning = false; /********************************** * Local Helper Functions protoypes **********************************/ -void BuildInitialDeviceList(); +static void BuildInitialDeviceList(); -void* ThreadFunc(void* ptr); -void WaitForDeviceHandled(); -void SignalDeviceHandled(); -void WaitForNewDevice(); -void SignalDeviceAvailable(); +static void WaitForDeviceHandled(); +static void SignalDeviceHandled(); +static void cbTerm(uv_signal_t *handle, int signum); +static void cbWork(uv_work_t *req); +static void cbAfter(uv_work_t *req, int status); +static void cbAsync(uv_async_t *handle); /********************************** * Public Functions **********************************/ -void NotifyAsync(uv_work_t* req) { - WaitForNewDevice(); -} - -void NotifyFinished(uv_work_t* req) { - if (isRunning) { - if (isAdded) { - NotifyAdded(currentItem); - } - else { - NotifyRemoved(currentItem); - } - } - - // Delete Item in case of removal - if(isAdded == false) { - delete currentItem; - } - - SignalDeviceHandled(); - uv_queue_work(uv_default_loop(), req, NotifyAsync, (uv_after_work_cb)NotifyFinished); -} - void Start() { isRunning = true; } void Stop() { isRunning = false; - pthread_mutex_lock(¬ify_mutex); - pthread_cond_signal(¬ifyNewDevice); - pthread_mutex_unlock(¬ify_mutex); } void InitDetection() { @@ -117,14 +96,13 @@ void InitDetection() { BuildInitialDeviceList(); - pthread_mutex_init(¬ify_mutex, NULL); - pthread_cond_init(¬ifyNewDevice, NULL); - pthread_cond_init(¬ifyDeviceHandled, NULL); - - uv_work_t* req = new uv_work_t(); - uv_queue_work(uv_default_loop(), req, NotifyAsync, (uv_after_work_cb)NotifyFinished); + uv_mutex_init(¬ify_mutex); + uv_async_init(uv_default_loop(), &async_handler, cbAsync); + uv_signal_init(uv_default_loop(), &term_signal); + uv_signal_init(uv_default_loop(), &int_signal); + uv_cond_init(¬ifyDeviceHandled); - pthread_create(&thread, NULL, ThreadFunc, NULL); + uv_queue_work(uv_default_loop(), &work_req, cbWork, cbAfter); Start(); } @@ -139,40 +117,23 @@ void EIO_Find(uv_work_t* req) { /********************************** * Local Functions **********************************/ -void WaitForDeviceHandled() { - pthread_mutex_lock(¬ify_mutex); +static void WaitForDeviceHandled() { + uv_mutex_lock(¬ify_mutex); if(deviceHandled == false) { - pthread_cond_wait(¬ifyDeviceHandled, ¬ify_mutex); + uv_cond_wait(¬ifyDeviceHandled, ¬ify_mutex); } deviceHandled = false; - pthread_mutex_unlock(¬ify_mutex); + uv_mutex_unlock(¬ify_mutex); } -void SignalDeviceHandled() { - pthread_mutex_lock(¬ify_mutex); +static void SignalDeviceHandled() { + uv_mutex_lock(¬ify_mutex); deviceHandled = true; - pthread_cond_signal(¬ifyDeviceHandled); - pthread_mutex_unlock(¬ify_mutex); -} - -void WaitForNewDevice() { - pthread_mutex_lock(¬ify_mutex); - if(newDeviceAvailable == false){ - pthread_cond_wait(¬ifyNewDevice, ¬ify_mutex); - } - newDeviceAvailable = false; - pthread_mutex_unlock(¬ify_mutex); + uv_cond_signal(¬ifyDeviceHandled); + uv_mutex_unlock(¬ify_mutex); } -void SignalDeviceAvailable() { - pthread_mutex_lock(¬ify_mutex); - newDeviceAvailable = true; - pthread_cond_signal(¬ifyNewDevice); - pthread_mutex_unlock(¬ify_mutex); -} - - - ListResultItem_t* GetProperties(struct udev_device* dev, ListResultItem_t* item) { +static ListResultItem_t* GetProperties(struct udev_device* dev, ListResultItem_t* item) { struct udev_list_entry* sysattrs; struct udev_list_entry* entry; sysattrs = udev_device_get_properties_list_entry(dev); @@ -199,7 +160,7 @@ void SignalDeviceAvailable() { return item; } -void DeviceAdded(struct udev_device* dev) { +static void DeviceAdded(struct udev_device* dev) { DeviceItem_t* item = new DeviceItem_t(); GetProperties(dev, &item->deviceParams); @@ -208,10 +169,10 @@ void DeviceAdded(struct udev_device* dev) { currentItem = &item->deviceParams; isAdded = true; - SignalDeviceAvailable(); + uv_async_send(&async_handler); } -void DeviceRemoved(struct udev_device* dev) { +static void DeviceRemoved(struct udev_device* dev) { ListResultItem_t* item = NULL; if(IsItemAlreadyStored((char *)udev_device_get_devnode(dev))) { @@ -231,14 +192,20 @@ void DeviceRemoved(struct udev_device* dev) { currentItem = item; isAdded = false; - SignalDeviceAvailable(); + uv_async_send(&async_handler); } -void* ThreadFunc(void* ptr) { - while (1) { - /* Make the call to receive the device. - select() ensured that this will not block. */ +static void cbWork(uv_work_t *req) { + uv_signal_start(&int_signal, cbTerm, SIGINT); + uv_signal_start(&term_signal, cbTerm, SIGTERM); + + pollfd fds = {fd, POLLIN, 0}; + while (!quit) { + int ret = poll(&fds, 1, 100); + if (!ret) continue; + if (ret < 0) break; + dev = udev_monitor_receive_device(mon); if (dev) { if(udev_device_get_devtype(dev) && strcmp(udev_device_get_devtype(dev), DEVICE_TYPE_DEVICE) == 0) { @@ -254,12 +221,43 @@ void* ThreadFunc(void* ptr) { udev_device_unref(dev); } } +} + +static void cbAfter(uv_work_t *req, int status) { + uv_signal_stop(&int_signal); + uv_signal_stop(&term_signal); + uv_close((uv_handle_t *) &async_handler, NULL); + uv_cond_destroy(¬ifyDeviceHandled); + uv_mutex_destroy(¬ify_mutex); + udev_monitor_unref(mon); + udev_unref(udev); +} + +static void cbAsync(uv_async_t *handle) { + if (isRunning) { + if (isAdded) { + NotifyAdded(currentItem); + } + else { + NotifyRemoved(currentItem); + } + } + + // Delete Item in case of removal + if(isAdded == false) { + delete currentItem; + } + + SignalDeviceHandled(); +} + - return NULL; +static void cbTerm(uv_signal_t *handle, int signum) { + quit = true; } -void BuildInitialDeviceList() { +static void BuildInitialDeviceList() { /* Create a list of the devices */ enumerate = udev_enumerate_new(udev); udev_enumerate_scan_devices(enumerate);