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

HandleScope::HandleScope Entering the V8 API without proper locking in place #118

Closed
legraphista opened this issue Sep 7, 2016 · 7 comments

Comments

@legraphista
Copy link

Hi,
Working with the following code:

const Threads = require('webworker-threads');

const threadPool = Threads.createPool(8);

function test (x){
    let xx = 0;
    for(let i = 0; i < 100000000; i++){
        xx = i;
    }
    console.log(`test ${x} ${xx}`);
}

threadPool.all.eval(test);

for(var i = 0; i < 100; i ++){
    threadPool.any.eval(`test(${i})`, (err) => console.error('callback', err.stack || err));
}

I can consistently reproduce the following error:

FATAL ERROR: HandleScope::HandleScope Entering the V8 API without proper locking in place
 1: node::Abort() [/Users/Stefan/.nvm/versions/node/v6.3.1/bin/node]
 2: node::FatalException(v8::Isolate*, v8::Local<v8::Value>, v8::Local<v8::Message>) [/Users/Stefan/.nvm/versions/node/v6.3.1/bin/node]
 3: v8::HandleScope::Initialize(v8::Isolate*) [/Users/Stefan/.nvm/versions/node/v6.3.1/bin/node]
 4: node::FatalException(v8::Isolate*, v8::TryCatch const&) [/Users/Stefan/.nvm/versions/node/v6.3.1/bin/node]
 5: Callback(uv_async_s*, int) [/Users/Stefan/Desktop/Work/node-playground/node_modules/webworker-threads/build/Release/WebWorkerThreads.node]
 6: uv__async_event [/Users/Stefan/.nvm/versions/node/v6.3.1/bin/node]
 7: uv__async_io [/Users/Stefan/.nvm/versions/node/v6.3.1/bin/node]
 8: uv__io_poll [/Users/Stefan/.nvm/versions/node/v6.3.1/bin/node]
 9: uv_run [/Users/Stefan/.nvm/versions/node/v6.3.1/bin/node]
10: node::Start(int, char**) [/Users/Stefan/.nvm/versions/node/v6.3.1/bin/node]
11: start [/Users/Stefan/.nvm/versions/node/v6.3.1/bin/node]
12: 0x4

Node: 4.5.0, 6.3.1, 6.5.0
OS: OSX 10.11.6

EDIT 1:
The issue was caused due to an error in the callback,err is null (silly me), so it would throw when accessing err.stack
I think this is still an issue because the error in the callback did not bubble up and did not appear, even with process.on('uncaughtException', (err)=>console.error(err)) present.

@audreyt audreyt added the bug label Sep 7, 2016
@brodycj brodycj added the testing label Nov 3, 2016
@brodycj
Copy link
Collaborator

brodycj commented Nov 3, 2016

Similar problem demonstrated by the following test case in test-package.js:

tap.test('eval pool with callback that throws', function(t) {
  var tcount = 0;
  function test() {
    console.log('test function in thread pool');
  }

  var mypool = Threads.createPool(5);
  //mypool.all.eval(test);
  mypool.all.eval('test()', function(err, data) {
    ++tcount;
    if (tcount === 2)
      throw new Error('boom');
    if (tcount == 5) {
      mypool.destroy();
      t.end();
    }
  });
});

@brodycj
Copy link
Collaborator

brodycj commented Nov 4, 2016

The following change (with JavaScript code regenerated by npm run js) seems to fix the problem with the original code but not in the test case above:

diff --git a/src/createPool.ls b/src/createPool.ls
index f12894b..f464560 100644
--- a/src/createPool.ls
+++ b/src/createPool.ls
@@ -41,7 +41,10 @@ function create-pool (n)
                     next-job t
                     f = job.cb-or-data
                     if typeof f is \function
-                      f.call t, e, d
+                      try
+                        f.call t, e, d
+                      catch e
+                        return e
                     else
                       t.emit job.src-text-or-event-type, f
             else if job.type is 2 # EMIT

Going to sleep now, hope to give it a try when I am up again.

@creole
Copy link

creole commented Dec 27, 2016

Any progress on this? Running into the exact same error.
Node: 6.9.2
OS: Debian GNU/Linux 8 (jessie)

@audreyt
Copy link
Owner

audreyt commented Dec 28, 2016

Fixed in webworker-threads-0.7.10. Thanks for the reminder!

@creole
Copy link

creole commented Dec 28, 2016

Thank you, will test later.

@davisjam
Copy link
Collaborator

@audreyt I must be missing something here. What does 0d9a167 actually accomplish? I see it must be important, since you added it again in 1a3fbed after I removed it in #141. But with RAII, what is the effect of creating a Locker within the scope of an otherwise empty if statement?

@davisjam
Copy link
Collaborator

@brodybits I don't see that test case in test-package.js. Since I appear to have inadvertently re-introduced this bug until @audreyt fixed it in 1a3fbed, perhaps adding it to the test suite would be helpful?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants