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

Fix segmentation fault on linux when calling stopMonitoring() #98

Closed
wants to merge 3 commits into from

Conversation

mrsch
Copy link

@mrsch mrsch commented Dec 3, 2019

udev_monitor_unref was called while cbWork was still accessing the monitor.
Fixed by moving to cbAfter.

Resolves #57

@mrsch
Copy link
Author

mrsch commented Dec 4, 2019

I added a cleanup hook to automatically stop monitoring and free the udev monitor when node exits.

@MadLittleMods
Copy link
Owner

MadLittleMods commented Apr 24, 2020

Sorry for the delay @mrsch 🙇

I'm unable to build on macOS and Node.js v12.16.2:

$ npm run rebuild

> [email protected] rebuild /Users/eric/Documents/github/node-usb-detection
> node-gyp rebuild

  CXX(target) Release/obj.target/detection/src/detection.o
../src/detection.cpp:224:9: warning: 'AtExit' is deprecated: Use the three-argument variant of AtExit() or AddEnvironmentCleanupHook()
      [-Wdeprecated-declarations]
                node::AtExit(CleanupDetection);
                      ^
/Users/eric/Library/Caches/node-gyp/12.16.2/include/node/node.h:700:1: note: 'AtExit' has been explicitly marked deprecated here
NODE_DEPRECATED(
^
/Users/eric/Library/Caches/node-gyp/12.16.2/include/node/node.h:101:20: note: expanded from macro 'NODE_DEPRECATED'
    __attribute__((deprecated(message))) declarator
                   ^
1 warning generated.
  CXX(target) Release/obj.target/detection/src/deviceList.o
  CXX(target) Release/obj.target/detection/src/detection_mac.o
  SOLINK_MODULE(target) Release/detection.node

@MadLittleMods
Copy link
Owner

Actually looks successful but just a warning about deprecation. I'm doing some Google'ing to see what the new thing to use is.

@MadLittleMods
Copy link
Owner

I'm not finding that much info about node::AtExit

It looks like it is supported in Node.js v10.x https://nodejs.org/docs/latest-v10.x/api/addons.html#addons_atexit_hooks

But then looking at Node.js v12 docs, it disappeared and I don't see in the deprecated API docs

In the Node.js v12 changelog, I see (SEMVER-MINOR) src: deprecate two- and one-argument AtExit() (Anna Henningsen) [#30227](https://github.com/nodejs/node/pull/30227)

From the warning, it says warning: 'AtExit' is deprecated: Use the three-argument variant of AtExit() or AddEnvironmentCleanupHook(). Wish I could find some documentation and examples around this.

I sorta see an example here

@MadLittleMods
Copy link
Owner

It seems like AddEnvironmentCleanupHook is the more favored option nowadays according to usage in the docs, https://nodejs.org/docs/latest-v12.x/api/addons.html

@mrsch What was the reason for switching away? dd4a240

@MadLittleMods
Copy link
Owner

Closing but willing to re-open when responding

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

Successfully merging this pull request may close these issues.

Calling startMonitoring() and stopMonitoring() multiple times causes segmentation fault (Linux, macOS)
2 participants