Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Don't restart polling when libuv emits an error #1804

Closed
wants to merge 1 commit into from
Closed

Fix: Don't restart polling when libuv emits an error #1804

wants to merge 1 commit into from

Conversation

nfrasser
Copy link

@nfrasser nfrasser commented Feb 14, 2019

Fixes #1803

This prevents an infinite loop of polling on a bad file handle

This prevents an infinite loop of polling on a bad file handle
Copy link
Member

@reconbot reconbot left a comment

Choose a reason for hiding this comment

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

This looks great but I want to test it with actual hardware. @HipsterBrown or @fivdi if either of you have any time to test please do.

@nfrasser
Copy link
Author

I am beta testing this fix with my fork and I'm finding that for some cases, the 'open' event never fires. The issue is specifically on macOS systems. Still working on getting the debug logs for those cases.

@reconbot any ideas as to why this might happen? It looks like the only option I have left it so force-restart the node process when we detect that serialport gets stuck.

@reconbot
Copy link
Member

This change doesn't effect port opening at all.

@@ -61,10 +61,10 @@ void Poller::onData(uv_poll_t* handle, int status, int events) {
// fprintf(stdout, "OnData status=%d events=%d\n", status, events);
argv[0] = Nan::Null();
argv[1] = Nan::New<v8::Integer>(events);
// remove triggered events from the poll
int newEvents = obj->events & ~events;
obj->poll(newEvents);
Copy link
Member

Choose a reason for hiding this comment

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

ok yes this wont update the poll events on an error - but no this wont stop the poller on an error we should try to call uv_poll_stop(poll_handle);

@reconbot
Copy link
Member

reconbot commented Jun 6, 2019

@nfrasser are you able to address the feedback?

@stale stale bot added the stale-issue label Aug 5, 2019
@serialport serialport deleted a comment from stale bot Aug 11, 2019
@stale stale bot removed the stale-issue label Aug 11, 2019
reconbot added a commit that referenced this pull request Sep 24, 2019
This will stop polling for FS events if there’s an error polling for fs events. Added tests for the JS. Tests for the c++ are harder.

This supersedes #1804 and ensures no more events are fired if one reports an error. Fixes #1803 based upon @nfrasser's work.
@reconbot
Copy link
Member

fixed in #1936

@reconbot reconbot closed this Sep 24, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Mar 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Should the Poller start polling the file handle again if it encounters an error?
2 participants