Skip to content
This repository has been archived by the owner on Mar 7, 2023. It is now read-only.

pthread to libuv, fix 100% CPU usage, some memory leaks #21

Closed
wants to merge 3 commits into from
Closed

pthread to libuv, fix 100% CPU usage, some memory leaks #21

wants to merge 3 commits into from

Conversation

sarakusha
Copy link
Contributor

@sarakusha sarakusha commented Dec 3, 2015

  • Migration from pthread to libuv.
  • Fix 100% CPU usage (using poll()):
    source
/**
 * 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
    udev_monitor_unref(mon);
    udev_unref(udev);
  • For local functions and variables using the keyword static

See #2

@sarakusha sarakusha mentioned this pull request Dec 3, 2015
@MadLittleMods
Copy link
Owner

Hey @sarakusha,

Sorry, I haven't had a chance to test this yet but have you tried it on Windows, Mac, and linux?

@sarakusha
Copy link
Contributor Author

I changed the src/detection_linux.cpp, so it should only affect Linux, if I understand you correctly. I am bad at English.

@blahah
Copy link

blahah commented Sep 29, 2016

Any chance of getting this merged? 100% cpu on linux systems would be nice to solve.

@MadLittleMods
Copy link
Owner

@blahah I haven't tested it yet. Does this PR fix the issue? What are the details of your OS?

@sarakusha
Copy link
Contributor Author

sarakusha commented Sep 30, 2016

I use this solution in production on ubuntu 14.10, 15.04, 15.10, 16.04 (x64)
computers are turned on around the clock (just somewhere 50 computers)

@gpgabriel
Copy link

gpgabriel commented May 24, 2017

I had the same 100% CPU issue on BananaPi and RaspberryPi 3 while using Debian Jessie with node v4.4 or v6.3 and it was fixed by merging this PR.

MadLittleMods added a commit that referenced this pull request Nov 9, 2017
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`
>
> #21 (comment)
MadLittleMods added a commit that referenced this pull request Nov 9, 2017
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`
>
> #21 (comment)
MadLittleMods added a commit that referenced this pull request Nov 9, 2017
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`
>
> #21 (comment)
MadLittleMods pushed a commit that referenced this pull request Nov 10, 2017
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`
>
> #21 (comment)
@MadLittleMods
Copy link
Owner

MadLittleMods commented Nov 10, 2017

Hey @sarakusha,

So sorry about the lack of attention on this 😞. I just tested this out again and it does fix the 100% CPU issue 💯. One of my reservations on not merging was not having this same new libuv thread pattern over on the macOS version. So I ported this PR on top of the latest master for Linux and macOS, #46 - I am also not very confident in my C++ to properly give this a good review.

One issue still present is things not wanting to exit gracefully (related to #35). And porting these changes over to macOS also introduced the same exiting issue to that platform (previously exited gracefully). This is reproducible if you run npm test, go through the manual steps and it just hangs (tested on Ubuntu 16.04.3 and macOS 10.12.6). If you unplug a USB device again while it is hanging, it will throw a segmentation fault. Granted, this problem wasn't introduced in this PR for Linux, I do want to maintain code parity with macOS and don't want to introduce the exit issue there. I'm not familiar enough with the libuv threading(or C++) to understand the issue here easily. Is this something you would be willing to take a look at again (working from #46)? 🙇

MadLittleMods pushed a commit that referenced this pull request Nov 12, 2017
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`
>
> #21 (comment)
MadLittleMods pushed a commit that referenced this pull request Nov 12, 2017
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`
>
> #21 (comment)
MadLittleMods pushed a commit that referenced this pull request Feb 4, 2018
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`
>
> #21 (comment)
@MadLittleMods
Copy link
Owner

Heads-up, #46 was merged and published in 2.1.0 🚀

Thanks again @sarakusha ❤️

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants