SnosMe / uiohook-napi

MIT License
162 stars 37 forks source link

Avoid crashing on JS exceptions when app is exiting #5

Closed hsource closed 6 months ago

hsource commented 3 years ago

Motivation

Crashes

While testing awakened-poe-trade on Mac, I noticed that quitting Awakened POE Trade would cause a crash instead of exiting gracefully.

To reproduce:

  1. Checkout the pull request for Mac at #403
  2. yarn electron:serve in awakened-poe-trade
  3. Open PathOfExile and use it for a little
  4. Use the tray to quit Awakened POE Trade

It'll give a stacktrace like this and an unexpected closing error:

FATAL ERROR: uiohook_to_js_event napi_define_properties
 1: 0x112458395 node::Buffer::New(v8::Isolate*, char*, unsigned long, void (*)(char*, void*), void*) [/Users/harry/code/awakened-poe-trade/node_modules/electron/dist/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
 2: 0x112458534 node::Buffer::New(v8::Isolate*, char*, unsigned long, void (*)(char*, void*), void*) [/Users/harry/code/awakened-poe-trade/node_modules/electron/dist/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
 3: 0x1124583b9 node::Buffer::New(v8::Isolate*, char*, unsigned long, void (*)(char*, void*), void*) [/Users/harry/code/awakened-poe-trade/node_modules/electron/dist/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
 4: 0x112431ac4 napi_fatal_error [/Users/harry/code/awakened-poe-trade/node_modules/electron/dist/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
 5: 0x119a0b236 uiohook_to_js_event [/Users/harry/code/awakened-poe-trade/node_modules/uiohook-napi/prebuilds/darwin-x64/node.napi.node]
 6: 0x119a0b271 tsfn_to_js_proxy [/Users/harry/code/awakened-poe-trade/node_modules/uiohook-napi/prebuilds/darwin-x64/node.napi.node]
 7: 0x1124343ae napi_ref_threadsafe_function [/Users/harry/code/awakened-poe-trade/node_modules/electron/dist/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
 8: 0x10fb1253f uv_idle_stop [/Users/harry/code/awakened-poe-trade/node_modules/electron/dist/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
 9: 0x10fb0c5ac uv_run [/Users/harry/code/awakened-poe-trade/node_modules/electron/dist/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]

Hang when exiting

Sometimes, when I close Awakened POE Trade via the dock, it fails to exit. It seems to do this only after the error message:

CGEventTap timeout!

Fix

  1. Added extra logging to errors where the status is not OK
    • We now also try to get the latest original error and add that to the printed message. From that, I saw that the error was actually caused by a pending exception
  2. Instead of crashing on pending exceptions, just return ASAP to allow the JS to take over, per the recommendations from Node.js docs
    • Added new macro to return on pending exceptions
  3. Added patch to libuiohook to avoid stopping the event thread. This prevents libuiohook from hanging and stopping the app from exiting

Testing

  1. Run yarn prebuild on local version
  2. yarn add ../uiohook-napi to install local version
  3. Run through same steps. No error occurs
SnosMe commented 2 years ago

Proposed changes will cause errors to be silently ignored. I'm actually confused why this happens, env is in napi_pending_exception state but when flow returns to js it's gone.

Test snippet:

const { uIOhook } = require('./dist')

uIOhook.on('mousemove', (e) => {
  console.log(`moved ${e.x} ${e.y}`)

  uIOhook.emit('error', new Error('test'))
})

uIOhook.start()
marcelblum commented 1 year ago

I'm also seeing this CGEventTap timeout! crash occur intermittently when synchronous/modal system dialogs are in use while uiohook-napi is running in Electron/Mac. Seems like it happens if a synchronous dialog is shown via dialog.showMessageBoxSync() and keyboard keys are pressed while the dialog is up. Doesn't happen totally consistently but the likelihood of it crashing seems to increase the longer the dialog remains onscreen. I'm able to mostly avoid it by making sure to call uIOhook.stop() just before spawning any sync dialog but still get the crash sometimes.

marcelblum commented 1 year ago

Still encountering the CGEventTap timeout! crash in an Electron app on macOS sometimes, especially on older/slower Intel Macs. Tends to happen when my Electron app is backgrounded and some intensive app is in the foreground with heavy cpu use. The result is that my app keeps running but the uiohook process stops working, and it's a real showstopper because AFAICT it's impossible for my app to detect when it enters this state, but then once it's in this state any call to uIOhook.stop() crashes the entire app.

@hsource I know this thread is old and this might have left your headspace long ago, but I was wondering what your fix here actually does on the CGEventTap timeout! event. I see that your changes prevent a crash but also seems to prevent the input listening process from restarting, is that correct? Seems like you were trying to simply avoid a crash on quitting, but the side effect is that if we're not quitting, the hook becomes disabled. If the CGEventTap timeout! occurs during normal use and not when quitting as in the case I described above, then uIOhook would have to be stopped and restarted (from JS) in order to regain key input listening?

marcelblum commented 1 year ago

I wonder if this recent commit to libuiohook 1.2 that changes memory allocation handling in create_event_runloop_info() and destroy_event_runloop_info() would help with this issue https://github.com/kwhat/libuiohook/pull/166

hsource commented 1 year ago

Sorry, I haven't touched this library in a while and don't think I'll have time to look into it more deeply. I'm not sure if it's related, and never really found the root cause. Your hint does seem possible!

My guess is that there's some race condition or a thrown error in native code that makes it get stuck. I haven't looked into it more deeply though.

marcelblum commented 1 year ago

Thanks for the reply @hsource, understood. If I get a chance I will try building uiohook-napi using latest libuiohook 1.2 and see if that change made a difference. Hopefully the next release of uiohook-napi will incorporate the change anyway. It's really hard to test for this issue as it's hard to trigger the conditions that lead to CGEventTap timeout! consistently.

mushan0x0 commented 1 year ago

I am also often troubled by this problem. The program stops working properly after a while. I hope this issue will be resolved soon.😵‍💫

komSh commented 11 months ago

I am also often troubled by this problem. The program stops working properly after a while. I hope this issue will be resolved soon.😵‍💫

Have this issue fixed, im facing this issue my app goes in not responding state in mac.

marcelblum commented 11 months ago

I noticed the recent release of uiohook-napi (v1.5.1) incorporates the above-mentioned fix in libuiohook (thanks @SnosMe!) but I haven't tested it on Mac yet. @mushan0x0 @komSh I wonder if you're still seeing this issue even in latest v1.5.1?

mushan0x0 commented 11 months ago

I noticed the recent release of uiohook-napi (v1.5.1) incorporates the above-mentioned fix in libuiohook (thanks @SnosMe!) but I haven't tested it on Mac yet. @mushan0x0 @komSh I wonder if you're still seeing this issue even in latest v1.5.1?

Thank you for your efforts. After testing, the issue still exists in v1.5.1.

marcelblum commented 10 months ago

Confirming that I also still get the occasional CGEventTap timeout! error in v1.5.1 which leads to silent failure of the hook and eventually crashes. Still really hard to trigger it though, tough to debug. Will try to investigate further when I have the time.

luke358 commented 6 months ago

same question,When running on a mac for a while exit software will not respond

hsource commented 6 months ago

The CGEventTap timeout error is fixed by #37 or https://github.com/kwhat/libuiohook/pull/184

This does not fix that issue, so I'll close this.