Skip to content

Commit

Permalink
fix: stop polling if the poller has an error (#1936)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
reconbot authored Sep 24, 2019
1 parent a5f7d60 commit c57b6e9
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 9 deletions.
10 changes: 5 additions & 5 deletions packages/bindings/lib/poller.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
const debug = require('debug')
const logger = debug('serialport/bindings/poller')
const EventEmitter = require('events')
const FDPoller = require('bindings')('bindings.node').Poller
const PollerBindings = require('bindings')('bindings.node').Poller

const EVENTS = {
UV_READABLE: 1,
UV_WRITABLE: 2,
UV_DISCONNECT: 4,
UV_READABLE: 0b0001,
UV_WRITABLE: 0b0010,
UV_DISCONNECT: 0b0100,
}

function handleEvent(error, eventFlag) {
Expand Down Expand Up @@ -35,7 +35,7 @@ function handleEvent(error, eventFlag) {
* Polls unix systems for readable or writable states of a file or serialport
*/
class Poller extends EventEmitter {
constructor(fd) {
constructor(fd, FDPoller = PollerBindings) {
logger('Creating poller')
super()
this.poller = new FDPoller(fd, handleEvent.bind(this))
Expand Down
60 changes: 60 additions & 0 deletions packages/bindings/lib/poller.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
const Poller = require('./poller')

class MockPollerBidnings {
constructor(fd, callback) {
this.fd = fd
this.callback = callback
}
poll(flag) {
this.lastPollFlag = flag
setImmediate(() => this.callback(null, flag))
}
}

class ErrorPollerBindings {
constructor(fd, callback) {
this.fd = fd
this.callback = callback
}
poll(flag) {
this.lastPollFlag = flag
setImmediate(() => this.callback(new Error('oh no!'), flag))
}
}

describe('Poller', () => {
it('constructs', () => {
new Poller(1, MockPollerBidnings)
})
it('can listen to the readable event', done => {
const poller = new Poller(1, MockPollerBidnings)
poller.once('readable', err => {
assert.equal(err, null)
assert.equal(poller.poller.lastPollFlag, 1)
done(err)
})
})
it('can listen to the writable event', done => {
const poller = new Poller(1, MockPollerBidnings)
poller.once('writable', err => {
assert.equal(err, null)
assert.equal(poller.poller.lastPollFlag, 2)
done(err)
})
})
it('can listen to the disconnect event', done => {
const poller = new Poller(1, MockPollerBidnings)
poller.once('disconnect', err => {
assert.equal(err, null)
assert.equal(poller.poller.lastPollFlag, 4)
done(err)
})
})
it('reports errors on callback', done => {
const poller = new Poller(1, ErrorPollerBindings)
poller.once('readable', err => {
assert.notEqual(err, null)
done()
})
})
})
15 changes: 11 additions & 4 deletions packages/bindings/src/poller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,22 +49,29 @@ void Poller::stop() {
}
}

int Poller::_stop() {
return uv_poll_stop(poll_handle);
}

void Poller::onData(uv_poll_t* handle, int status, int events) {
Nan::HandleScope scope;
Poller* obj = static_cast<Poller*>(handle->data);
v8::Local<v8::Value> argv[2];

// if Error
if (0 != status) {
// fprintf(stdout, "OnData Error status=%s events=%d\n", uv_strerror(status), events);
argv[0] = v8::Exception::Error(Nan::New<v8::String>(uv_strerror(status)).ToLocalChecked());
argv[1] = Nan::Undefined();
obj->_stop(); // doesn't matter if this errors
} else {
// fprintf(stdout, "OnData status=%d events=%d\n", status, events);
// fprintf(stdout, "OnData status=%d events=%d subscribed=%d\n", status, events, obj->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);
}
// remove triggered events from the poll
int newEvents = obj->events & ~events;
obj->poll(newEvents);

obj->callback.Call(2, argv, obj);
}
Expand Down
1 change: 1 addition & 0 deletions packages/bindings/src/poller.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class Poller : public Nan::ObjectWrap, public Nan::AsyncResource {
~Poller();
void poll(int events);
void stop();
int _stop();

static NAN_METHOD(New);
static NAN_METHOD(poll);
Expand Down

0 comments on commit c57b6e9

Please sign in to comment.