audreyt / node-webworker-threads

Lightweight Web Worker API implementation with native threads
https://npmjs.org/package/webworker-threads
Other
2.3k stars 149 forks source link

It locks promise's `resolv()` inside mocha tests. #123

Open aurium opened 8 years ago

aurium commented 8 years ago

Hi folks. That is a weird bug or a big mistake. I need your help to solve anyway. I'm trying to make synaptic.js to work in parallel in node.js, and @Jabher point me this project to make the job without local WebWorker re-implementation. The problem is: with webworker-threads the mocha test freezes until timeout.

I did simplify the test to point you where it happens without synaptic.js code:

describe('Weird Things', function () {
  this.timeout(5000);

  it('Listen wwthreads message', function (done) {
    var Worker = require('webworker-threads').Worker;
    w = new Worker(new Function('postMessage("t1: OK")'));
    w.onmessage = function(msg){
      console.log('t1: Mesage:',msg);
      done();
      console.log('t1: done() was called.');
    };
  });

  it("Promise resolved by timeout", function (done) {
    new Promise(function(resolve, reject) {
      try {
        setTimeout(function(){
          console.log('t2: Timeout.');
          resolve();
          console.log('t2: resolve() was called.');
        }, 100);
      } catch(e) {
        reject(e)
      }
    }).then(function(){ done(); console.log('t2: then() was called') });
  });

  it("Promise resolved by listen wwthreads message", function (done) {
    new Promise(function(resolve, reject) {
      try {
        var Worker = require('webworker-threads').Worker;
        w = new Worker(new Function('postMessage("t3: OK")'));
        w.onmessage = function(msg){
          console.log('t3: Mesage:',msg);
          resolve();
          console.log('t3: resolve() was called.');
        };
      } catch(e) {
        reject(e)
      }
    }).then(function(){ done(); console.log('t3: then() was called') });
  });

});

And it outputs:

  Weird Things
t1: Mesage: { data: 't1: OK' }
    ✓ Listen wwthreads message
t1: done() was called.
t2: Timeout.
t2: resolve() was called.
    ✓ Promise resolved by timeout (105ms)
t2: then() was called
t3: Mesage: { data: 't3: OK' }
t3: resolve() was called.
    1) Promise resolved by listen wwthreads message
t3: then() was called

  2 passing (5s)
  1 failing

  1) Weird Things Promise resolved by listen wwthreads message:
     Error: timeout of 5000ms exceeded. Ensure the done() callback is being called in this test.

As you can see, only when we mix Promise with webworker-threads the problem appear, and the then() was called, only after the timeout, doesn't matter how long it is.

You can see why it need this test to work at Trainer::trainAsync(). I believe that can be a problem to many other possible webworker-threads uses. That is not a problem if i implement the same feature with child_process module mimicking WebWorkers.

Thanks.

EduardoRFS commented 8 years ago

When event queue is empty, node eventloop close, webworker need to keep events in queue, like cluster module

brodycj commented 8 years ago

I think this needs documentation.

aurium commented 8 years ago

I believe it is not a open question, it is a bug. @EduardoRFS makes a god point on event queue.

tswaters commented 7 years ago

See the following issue: https://github.com/audreyt/node-webworker-threads/issues/93 -- basically, wrap with setImmediate

I had a similar issue first time using this module. Had something like this:

return new Promise((resolve, reject) => {
  t.eval('fibo(10)', (err, result) => {
    if (err) { return reject(err) }
    resolve(result)
  })
})

And it would hang for sometimes upwards of 30 seconds before responding. I thought it was theads being weird, building on windows, something.... changed it to wrap the resolution/rejection in setImmediate and it works much better

return new Promise((resolve, reject) => {
  t.eval('fibo(10)', (err, result) => {
    setImmediate(() => {
      if (err) { return reject(err) }
      resolve(result)
    })
  })
})
joshjung commented 5 years ago

Still having this issue over a year later :(

audreyt commented 5 years ago

Hi! This module's functionality is now part of Node core https://nodejs.org/api/worker_threads.html

If you would like, I wonder if you (or someone interested) can help create a compatibility API on top of the core Node API and thus make this extension a polyfill only for earlier Node versions?