Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

CHECK(entry_stack_ == __null || entry_stack_->previous_item == __null) failed #158

Closed
bnoordhuis opened this Issue · 13 comments

4 participants

@bnoordhuis

With joyent/node@d58c206 (v0.11.12-pre):

var F = require('../build/Debug/fibers');

var a = F.Fiber(function() {
  console.log('>> a');
  F.yield();
  console.log('<< a');
});

var b = F.Fiber(function() {
  console.log('>> b');
  F.yield();
  console.log('<< b');
});

a.run();
b.run();
return;  // Removing the return fixes it.
a.run();
b.run();

Needs a debug build because the CHECK is really an ASSERT.

$ ../master/out/Debug/node tmp/t.js
>> a
>> b


#
# Fatal error in ../deps/v8/src/isolate.cc, line 1978
# CHECK(entry_stack_ == __null || entry_stack_->previous_item == __null) failed
#

==== C stack trace ===============================

 1: V8_Fatal
 2: v8::internal::Isolate::~Isolate()
 3: v8::internal::V8::TearDown()
 4: v8::V8::Dispose()
 5: node::Start(int, char**)
 6: main
 7: __libc_start_main
 8: ??
Trace/breakpoint trap (core dumped)

When you remove the return statement both fibers run to completion and the entry stack is restored to normalcy.

I'm not sure whether it's actually fixable but if you manage to get it working, please let me know - I'm grappling with a similar issue in another project. :-)

@laverdet
Owner

I'm so sorry. My github emails haven't been coming through for months so I'm just now seeing this. Did you make any progress? I'm getting started right now but it looks hairy ;)

@laverdet
Owner

Well, at least it's a problem in V8::Dispose() so it doesn't affect anything besides return codes and nasty output when your program exits.

@laverdet
Owner

Yeah so I guess the issue here is that v8 doesn't like it if you try to shut it down and there are still resources in use that it can't release (the Lockers). This seems like reasonable behavior. I'm not certain what the correct behavior here for fibers should be. It's going to add a lot of bookkeeping to manually keep track of every fiber (currently v8 does this for us). Perhaps I should just keep a total number of currently active fibers and hook when node is exiting and throw a warning..

@laverdet
Owner

As for why it's ok with 1 fiber, but not 2.. that remains a mystery.

@bnoordhuis

Perhaps I should just keep a total number of currently active fibers and hook when node is exiting and throw a warning..

That sounds like a reasonable solution.

For something completely different, the V8 people are planning some changes that I think will break node-fibers bad. Here is the email exchange: https://groups.google.com/d/msg/v8-users/wyhgmD_f7C4/b38ObR_HGe0J

@laverdet
Owner

For something completely different, the V8 people are planning some changes that I think will break node-fibers bad. Here is the email exchange: https://groups.google.com/d/msg/v8-users/wyhgmD_f7C4/b38ObR_HGe0J

I think I can handle it. There's always a flag in memory somewhere, and I will be able to find it. But it does sound like the changes they have in mind will actually allow me to remove the TLS hacks and have fibers be "legit".

@rosen-vladimirov

Hi,

I think the problem is more serious than expected. We are using Fibers in one of our projects and we have problems when using Future.wait(). I've tried to simplify the code and here is one script that will crash on windows 8.1 without finishing its work:

var Fiber = require("fibers");
var Future = require("fibers/future");

var createFiber = Fiber(function() {
    function sleep(ms) {
        var future = new Future;
        setTimeout(function() {
            future.return();
        }, ms);
        return future;
    }

    var doSomeHardWork = function(ms) {
        sleep(ms).wait();
        console.log("      -->Finished with doSomeHardWork.");
    }.future(); 

    var calcTimerDelta = function(ms) {
        var s = new Date
        sleep(ms).wait();
        var futs = [];
        for(var j = 0; j < 3; j++) {
            console.log("   --> Creating future from doSomeHardWork. Iteration #" + j);
            var fut = doSomeHardWork(1000);
            futs.push(fut);
        }

        console.log("   --> Waiting for futures from doSomeHardWork: they are " + futs.length);
        Future.wait(futs);
        console.log("   --> Finished with futures from doSomeHardWork");
        var d = new Date - s;
        console.log("   --> Finished with calctimerDelta: d = " + d);
    }.future(); 

    //sleep(10000).wait();
    for(var i = 0; i < 3; i++) {
        console.log("\x1B[31;1m" + "Creating future from calcTimerDelta. Iteration #" + i + "\x1B[0m");
        var future = calcTimerDelta(1000);
        future.wait();
    }

    console.log("Finished with all futures!");
});

console.log("Run Fiber, Run!");
createFiber.run();

After some hard time of debugging it turned out in coroutine.cc you are expecting to have isolate_key_, thread_id_key_ and per_isolate_thread_data_key_ (declared in v8's isolate.cc) to have consecutive values (for example 11, 12, 13). In case they are not consecutive, the code crashes. On our machines we executed the following:

#include "stdafx.h"
#include <Windows.h>
#include <iostream>

int _tmain(int argc, _TCHAR* argv[])
{
    for (int i = 0; i < 50; ++i)
    {
        std::cout << TlsAlloc() << std::endl;
    }
    return 0;
}

which led to surprise - index 16 is missing on windows 8.1. We've debugged isolate.cc code and found that for isolate_key_ we receive index 14, for thread_id_key_ - index 15 and for per_isolate_thread_data_key_ - index 17. This leads to the unexpected behavior in coroutine.cc.

Do you have any suggestions how to fix this? Can we use better way in coroutine.cc in order to find the indexes instead of just believing they are consecutive values? (I can open a separate issue in case you think it's required).

Thanks in advance for your help.

@laverdet
Owner

@rosen-vladimirov first off, great detective work. Thanks so much for the detailed report, that's all super helpful.

Could you try applying this patch, recompiling, and letting me know if it fixes it?

https://gist.github.com/laverdet/26da7e27efa9934fcd2d

@rosen-vladimirov

@laverdet, thanks for the quick response. I've used your fix and tried building, but without success. Here's the log from node-gyp rebuild:
D:\Work\fibersTests\node_modules\fibers>node-gyp rebuild
gyp info it worked if it ends with ok
gyp info using node-gyp@1.0.2
gyp info using node@0.10.33 | win32 | ia32
gyp info spawn python
gyp info spawn args [ 'C:\Users\vladimirov\AppData\Roaming\npm\node_modules\node-gyp\gyp\gyp_main.py',
gyp info spawn args 'binding.gyp',
gyp info spawn args '-f',
gyp info spawn args 'msvs',
gyp info spawn args '-G',
gyp info spawn args 'msvs_version=auto',
gyp info spawn args '-I',
gyp info spawn args 'D:\Work\fibersTests\node_modules\fibers\build\config.gypi',
gyp info spawn args '-I',
gyp info spawn args 'C:\Users\vladimirov\AppData\Roaming\npm\node_modules\node-gyp\addon.gypi',
gyp info spawn args '-I',
gyp info spawn args 'C:\Users\vladimirov\.node-gyp\0.10.33\common.gypi',
gyp info spawn args '-Dlibrary=shared_library',
gyp info spawn args '-Dvisibility=default',
gyp info spawn args '-Dnode_root_dir=C:\Users\vladimirov\.node-gyp\0.10.33',
gyp info spawn args '-Dmodule_root_dir=D:\Work\fibersTests\node_modules\fibers',
gyp info spawn args '--depth=.',
gyp info spawn args '--no-parallel',
gyp info spawn args '--generator-output',
gyp info spawn args 'D:\Work\fibersTests\node_modules\fibers\build',
gyp info spawn args '-Goutput_dir=.' ]
gyp info spawn C:\Program Files (x86)\MSBuild\12.0\bin\msbuild.exe
gyp info spawn args [ 'build/binding.sln',
gyp info spawn args '/clp:Verbosity=minimal',
gyp info spawn args '/nologo',
gyp info spawn args '/p:Configuration=Release;Platform=Win32' ]
Building the projects in this solution one at a time. To enable parallel build, please add the "/m" switch.
fibers.cc
coroutine.cc
..\src\coroutine.cc(36): warning C4146: unary minus operator applied to unsigned type, result still unsigned [D:\Work\fibersTests\node_modules\fibers\build\fibers.vcxproj]
C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\include\xlocale(323): warning C4530: C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc [D:\Work\fibersTests\node_modules\fibers\build\fibers.vcxproj]
C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\include\vector(753): warning C4530: C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc [D:\Work\fibersTests\node_modules\fibers\build\fibers.vcxproj]
C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\include\vector(746) : while compiling class template member function 'void std::vector<Ty>::reserve(unsigned int)'
with
[
_Ty=Coroutine *
]
..\src\coroutine.cc(27) : see reference to class template instantiation 'std::vector<_Ty>' being compiled
with
[
_Ty=Coroutine *
]
C:\Users\vladimirov.node-gyp\0.10.33\deps\v8\include\v8.h(179): warning C4506: no definition for inline function 'v8::Persistent v8::Persistent::New(v8::Handle)' [D:\Work\fibersTests\node_modules\fibers\build\fibers.vcxproj]
with
[
T=v8::Object
]
..\src\fibers.cc(247): warning C4244: 'argument' : conversion from 'int64_t' to 'intptr_t', possible loss of data [D:\Work\fibersTests\node_modules\fibers\build\fibers.vcxproj]
..\src\fibers.cc(707): warning C4244: '=' : conversion from 'double' to 'size_t', possible loss of data [D:\Work\fibersTests\node_modules\fibers\build\fibers.vcxproj]
C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\include\xmemory(196): warning C4506: no definition for inline function 'v8::Persistent v8::Persistent::New(v8::Handle)' [D:\Work\fibersTests\node_modules\fibers\build\fibers.v cxproj]
with
[
T=v8::Object
]
C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\include\xmemory(196): warning C4506: no definition for inline function 'v8::Persistent v8::Persistent::New(v8::Handle)' [D:\Work\fibersTests\node_modules\fibers\build\fibers.v cxproj]
with
[
T=v8::FunctionTemplate
]
coro.c
Creating library D:\Work\fibersTests\node_modules\fibers\build\Release\fibers.lib and object D:\Work\fibersTests\node_modules\fibers\build\Release\fibers.exp
coroutine.obj : error LNK2001: unresolved external symbol "public: static unsigned int v8::internal::Isolate::per_isolate_thread_data_key
" (?per_isolate_thread_data_key_@Isolate@internal@v8@@2IA) [D:\Work\fibersTests\node_modules\fibers\b uild\fibers.vcxproj]
coroutine.obj : error LNK2001: unresolved external symbol "public: static unsigned int v8::internal::Isolate::thread_id_key_" (?thread_id_key_@Isolate@internal@v8@@2IA) [D:\Work\fibersTests\node_modules\fibers\build\fibers.vcxproj]
coroutine.obj : error LNK2001: unresolved external symbol "public: static unsigned int v8::internal::Isolate::isolate_key_" (?isolate_key_@Isolate@internal@v8@@2IA) [D:\Work\fibersTests\node_modules\fibers\build\fibers.vcxproj]
D:\Work\fibersTests\node_modules\fibers\build\Release\fibers.node : fatal error LNK1120: 3 unresolved externals [D:\Work\fibersTests\node_modules\fibers\build\fibers.vcxproj]
gyp ERR! build error
gyp ERR! stack Error: C:\Program Files (x86)\MSBuild\12.0\bin\msbuild.exe failed with exit code: 1
gyp ERR! stack at ChildProcess.onExit (C:\Users\vladimirov\AppData\Roaming\npm\node_modules\node-gyp\lib\build.js:267:23)
gyp ERR! stack at ChildProcess.emit (events.js:98:17)
gyp ERR! stack at Process.ChildProcess._handle.onexit (child_process.js:810:12)
gyp ERR! System Windows_NT 6.2.9200
gyp ERR! command "node" "C:\Users\vladimirov\AppData\Roaming\npm\node_modules\node-gyp\bin\node-gyp.js" "rebuild"
gyp ERR! cwd D:\Work\fibersTests\node_modules\fibers
gyp ERR! node -v v0.10.33
gyp ERR! node-gyp -v v1.0.2
gyp ERR! not ok

@rosen-vladimirov

@laverdet, thank you very much for the new fix. I've tried it and everything seems to work fine. I've successfully rebuild and executed my code, which was failing - now it is executed correctly.

@laverdet laverdet closed this issue from a commit
@laverdet Fix TLS detection on Windows 8.1
The assumption that v8's TLS keys are all together in a row doesn't hold
true 100% of the time in Windows, it seems. This patch includes a new
TLS searching method that finds all the keys with a different heuristic.

Fixes #158 (@rosen-vladimirov's comment)
a3fd362
@laverdet laverdet closed this in a3fd362
@laverdet
Owner

Great! The new version is on npm now as well.

@tristanz

Unfortunately it looks like this introduces bug #197.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.