Some weird crash in extreme cases

[valera-rozuvan]

I am getting the following error is some cases. I haven't been able to pin point exactly when.

(node:21139) UnhandledPromiseRejectionWarning: Error [ERR_STREAM_DESTROYED]: Cannot call write after a stream was destroyed
    at doWrite (_stream_writable.js:406:19)
    at writeOrBuffer (_stream_writable.js:394:5)
    at Socket.Writable.write (_stream_writable.js:294:11)
    at e.value (/home/ubuntu/dev/proj-01/node_modules/node-uci/lib/index.js:1:152712)
    at e.<anonymous> (/home/ubuntu/dev/proj-01/node_modules/node-uci/lib/index.js:1:158159)
    at b (/home/ubuntu/dev/proj-01/node_modules/node-uci/lib/index.js:1:10260)
    at Generator._invoke (/home/ubuntu/dev/proj-01/node_modules/node-uci/lib/index.js:1:10048)
    at Generator.e.(anonymous function) [as next] (/home/ubuntu/dev/proj-01/node_modules/node-uci/lib/index.js:1:10439)
    at o (/home/ubuntu/dev/proj-01/node_modules/node-uci/lib/index.js:1:25705)
    at /home/ubuntu/dev/proj-01/node_modules/node-uci/lib/index.js:1:25846
    at new Promise (<anonymous>)
    at new t (/home/ubuntu/dev/proj-01/node_modules/node-uci/lib/index.js:1:5155)
    at e.<anonymous> (/home/ubuntu/dev/proj-01/node_modules/node-uci/lib/index.js:1:25640)
    at e.<anonymous> (/home/ubuntu/dev/proj-01/node_modules/node-uci/lib/index.js:1:158435)
    at Timeout.setTimeout [as _onTimeout] (/home/ubuntu/dev/proj-01/main.js:99:33)
    at ontimeout (timers.js:436:11)
(node:21139) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
(node:21139) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Do you have any idea what this might be?

[valera-rozuvan]

After further inspection, this happens when i call await engine.stop(). But only 1 in 100 cases (+- 10).

[valera-rozuvan]

I see 2 potential places where we are calling write, but the proc could already be non existent:

1. https://github.com/ebemunk/node-uci/blob/master/src/Engine/index.js#L236
2. https://github.com/ebemunk/node-uci/blob/master/src/Engine/index.js#L474

Is it possible that the process dies, and we reach one of this points in the code? Because inside the actual write function we do not check for proc...

[valera-rozuvan]

I think there is a race condition in function quit ( https://github.com/ebemunk/node-uci/blob/master/src/Engine/index.js#L230 ), and in 0.1% cases it goes the wrong way, and we have an error.

[ebemunk]

Thanks for the report @valera-rozuvan, indeed there are checks for the existence of proc before the 2 write calls, but the error you're seeing does describe that the stream was destroyed before the write is issued, despite the check.

can you provide some code that reproduces this (even in the 0.1% cases)? i suspect it might also be related to how you call quit, certainly smells like a race condition

[valera-rozuvan]

const engine = new Engine(enginePath);

await engine.init();
await engine.isready();
await engine.setoption('Threads', 5);
await engine.setoption('MultiPV', 5);
await engine.position(fenStr);

const emitter = engine.goInfinite();

setTimeout(async () => {
  const result = await engine.stop();
  
  outputResult(result);

  await engine.quit();
}, 400);

emitter.on('data', (result) => {
  // console.log('---------------------------------------');
  // console.log(result);
  // console.log('\n\n\n');
});

[valera-rozuvan]

I suspect that the race condition happens when the engine process exits unexpectedly before (or during) the call to engine.quit(). Since this happens very rarely (engine is Stockfish), I can't reproduce it 100%.

[valera-rozuvan]

We need to analyze the function quit(). What happens if the engine process exits unexpectedly while the JS runtime is executing the first statement of the function? The second line? ... Case by case. Maybe put a generic try { ... } catch (e) { ... }?

[valera-rozuvan]

I am worried about the future Node.js versions and the warning:

DeprecationWarning: Unhandled promise rejections are deprecated.
In the future, promise rejections that are not handled will terminate
the Node.js process with a non-zero exit code.

[valera-rozuvan]

I believe that the function:

write(command) {
    this.proc.stdin.write(`${command}${EOL}`)
    engineLog('<-', command, EOL)
}

should include a try catch block around the call to this.proc.stdin.write. This is the safest IMHO :smile:

[ebemunk]

Good suggestions here, I will look into it when I get a chance. There should certainly be no unhandled exceptions.

[kurko]

The PR above should fix the problem with quit().