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

Fix 100% CPU usage, use non-blocking IO to exit gracefully (remove side-effects) #46

Merged
merged 7 commits into from
Feb 7, 2018

Conversation

MadLittleMods
Copy link
Owner

@MadLittleMods MadLittleMods commented Nov 9, 2017

Mergeable version of #21 from @sarakusha

Todo:

  • Port new detection_linux.cpp app flow to detection_mac.cpp
  • Figure out things not wanting to exit gracefully on Linux and macOS (related to Process won't exit on Linux #35)
    • This is reproducible if you run npm test, go through the manual steps and it just hangs. If you unplug a USB device again, it will throw a segmentation fault.
    • Using usbDetect.stopMonitoring() has no effect

Testing

  • Windows 10 (Fall Creators Update)
  • macOS 10.12.6
  • Ubuntu 16.04

Reference


Test Ubuntu on Windows 10 with Hyper-V enabled

For reference, here is how I tested this PR on Windows 10 with Hyper-V in a Ubuntu 16.04.3 VM.

VirtualBox will BSOD your PC if you have Hyper-V enabled. Otherwise, we could use their slick USB device sharing.

@MadLittleMods MadLittleMods force-pushed the mergeable-pr-21-fix-linux-blocking branch 3 times, most recently from 4de8ff5 to 78e3948 Compare November 10, 2017 00:07
@MadLittleMods MadLittleMods changed the title Fix 100% CPU usage, use non-blocking IO, accurate exit Fix 100% CPU usage, use non-blocking IO Nov 10, 2017
uv_close((uv_handle_t *) &async_handler, NULL);
uv_cond_destroy(&notifyDeviceHandled);
uv_mutex_destroy(&notify_mutex);
CFRunLoopStop(CFRunLoopGetCurrent());
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not confident in this CFRunLoopStop call here (kinda guessing on whether it is necessary)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be CFRunLoopStop(gRunLoop);

MadLittleMods and others added 3 commits February 4, 2018 15:10
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 MadLittleMods force-pushed the mergeable-pr-21-fix-linux-blocking branch 2 times, most recently from 2f3efad to 7ff88c5 Compare February 4, 2018 23:18
isRunning = false;

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might need to delete threadHandle 🤔

@MadLittleMods MadLittleMods force-pushed the mergeable-pr-21-fix-linux-blocking branch from 7ff88c5 to 438a5bb Compare February 5, 2018 03:28
@MadLittleMods MadLittleMods force-pushed the mergeable-pr-21-fix-linux-blocking branch from 438a5bb to 2ef3fb7 Compare February 5, 2018 03:42
@MadLittleMods MadLittleMods force-pushed the mergeable-pr-21-fix-linux-blocking branch 3 times, most recently from 476f1e5 to aa63ea8 Compare February 5, 2018 08:02
@MadLittleMods MadLittleMods force-pushed the mergeable-pr-21-fix-linux-blocking branch from aa63ea8 to a48f0c6 Compare February 5, 2018 08:23
@MadLittleMods MadLittleMods changed the title Fix 100% CPU usage, use non-blocking IO Fix 100% CPU usage, use non-blocking IO to exit gracefully (remove side-effects) Feb 7, 2018
@MadLittleMods MadLittleMods merged commit 6fdc57e into master Feb 7, 2018
@MadLittleMods MadLittleMods deleted the mergeable-pr-21-fix-linux-blocking branch May 14, 2018 08:52
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.

Process won't exit on Linux CPU loaded 100%
2 participants