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

Timing issue can cause EIO_AfterWrite to throw exception #639

Closed
bminer opened this issue Dec 1, 2015 · 4 comments
Closed

Timing issue can cause EIO_AfterWrite to throw exception #639

bminer opened this issue Dec 1, 2015 · 4 comments

Comments

@bminer
Copy link

bminer commented Dec 1, 2015

In some cases, calling write() while the SerialPort is being closed and a concurrent write is in progress can result in an uncaught exception. I'm not exactly sure why this is happening, but here's my hunch: the 2nd write is queued until the 1st write completes, and when the 2nd write is dequeued after the SerialPort has already closed, we have an exception.

TypeError: There's no write queue for that file descriptor (after write)!
    at TypeError (native)

This error is apparently thrown in EIO_AfterWrite. Even if the SerialPort has an "error" event handler registered and even if the call to write(buf, callback) has a callback specified, this error is still thrown as an uncaught exception.

Code to reproduce:

var SerialPort = require("serialport").SerialPort;
var port = new SerialPort("/dev/ttyUSB0");
port.on("error", function(e) {
    console.log("error event", e.stack);
});
port.on("open", function() {
    // Close the SerialPort after a short period of time
    setTimeout(function() {
        port.close();
    }, 10);
    // Start writing data
    write(30);
});
function write(timesLeft) {
    if(timesLeft <= 0) return;
    console.log("write", timesLeft);
    try {
        if(port.isOpen() ) {
            port.write(new Buffer([1, 2, 3, 4, 5]), function(err) {
                console.log("write callback", err ? err : "no error");
            });
        }
    } catch(e) {
        console.log("catch error", e.stack);
    }
    /* Note that we didn't wait for the write to complete before we call
        `write(timesLeft)` again. Chances are that 1 ms. isn't going to be long
        enough for the write to finish, causing the next write to be queued */
    setTimeout(write.bind(null, timesLeft - 1), 1);
}

Sample output from the above program looks like this:

write 30
write 29
write callback no error
write callback [Error: Error Bad file descriptor calling write(...)]
undefined:0



TypeError: There's no write queue for that file descriptor (after write)!
    at TypeError (native)

Interestingly, undefined:0 is printed from somewhere? It doesn't appear to be a call to console.log or console.error either... Then, of course, the TypeError is thrown and stack trace is printed.

Note that the exception from EIO_AfterWrite does not occur if you do not pass a callback to the write() routine. That is because of #638; the program will instead crash beforehand due to an Unhandled 'error' event.

Also note that the exception from EIO_AfterWrite does not occur if the timing is changed. This bug might not occur on all systems, and the behavior might change if the baud rate is changed. For example, if the delay to close the serial port is increased to 100 ms. and the delay between queued writes is increased to 10 ms, this bug occurs about 50% of the time.

Anyway, I'd be happy to provide more information about this bug if needed. The fix for this isn't very obvious to me because I'm not familiar with Node addons or libuv, but I'd be happy to help as much as possible.

@bminer
Copy link
Author

bminer commented Dec 1, 2015

A workaround for this bug is to prevent write() calls if closing property is true.

@reconbot
Copy link
Member

reconbot commented Apr 6, 2016

I think this is fixed but I'll run your code and check it out.

@reconbot
Copy link
Member

reconbot commented Apr 6, 2016

I can confirm this bug is fixed in #733 [email protected]

write 30
write 29
write 28
write callback no error
write callback no error
write callback no error
write 27
write 26
write 25
write 24
write 23
write 22
write 21
write 20
write 19
write 18
write 17
write 16
write 15
write 14
write 13
write 12
write 11
write 10
write 9
write 8
write 7
write 6
write 5
write 4
write 3
write 2
write 1

Please reopen if you're having any issues with the new version! Testing it out would help a lot! Thanks!

@reconbot reconbot closed this as completed Apr 6, 2016
@bminer
Copy link
Author

bminer commented Apr 13, 2016

@reconbot - Thanks so much! I will test out the new version ASAP and will write back if any issues occur.

@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

2 participants