debugger: guard against call from non-node context

[bnoordhuis]

Fix a segmentation fault when the debug message handler was called from
a context without an associated node::Environment.

Fixes: #4261
Fixes: #4322

R=@indutny?

CI: https://ci.nodejs.org/job/node-test-pull-request/1017/

[evanlucas]

CI looks happy minus a flaky ubuntu test.

LGTM

[jasnell]

LGTM

[jasnell]

Landed in 25776f3

[indutny]

Belated LGTM

[MylesBorins]

Is this critical enough to be rushed into 4.2.5?

/cc @jasnell@bnoordhuis

[Trott]

FWIW, the test is flaky on Windows 10 in CI. Not sure why or what the fix might be. See #4343.

[bnoordhuis]

The change itself is low risk and quite a few people reported it so I'd say it's a good candidate for LTS. I'll take a look at #4343.

[andyburke]

I believe I am seeing this crash in 4.3.0:

PID 33 received SIGSEGV for address: 0x3a8
/usr/local/lib/node_modules/segfault-handler/build/Release/segfault-handler.node(+0x1af4)[0x7fae9c2dfaf4]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x10340)[0x7faea00be340]
node(_ZN4node8debugger5Agent14MessageHandlerERKN2v85Debug7MessageE+0x41)[0xddf351]
node(_ZN2v88internal5Debug20NotifyMessageHandlerENS_10DebugEventENS0_6HandleINS0_8JSObjectEEES5_b+0x7d0)[0xa3a850]
node(_ZN2v88internal5Debug17ProcessDebugEventENS_10DebugEventENS0_6HandleINS0_8JSObjectEEEb+0x93)[0xa3ae43]
node(_ZN2v88internal5Debug14OnAfterCompileENS0_6HandleINS0_6ScriptEEE+0x334)[0xa3c754]
node[0xa1aad8]
node(_ZN2v88internal8Compiler13CompileScriptENS0_6HandleINS0_6StringEEENS2_INS0_6ObjectEEEiiNS_19ScriptOriginOptionsES6_NS2_INS0_7ContextEEEPNS_9ExtensionEPPNS0_10ScriptDataENS_14ScriptCompiler14CompileOptionsENS0_11NativesFlagEb+0x4ce)[0xa1deae]
node(_ZN2v814ScriptCompiler22CompileUnboundInternalEPNS_7IsolateEPNS0_6SourceENS0_14CompileOptionsEb+0x1ef)[0x8dd26f]
node(_ZN2v814ScriptCompiler14CompileUnboundEPNS_7IsolateEPNS0_6SourceENS0_14CompileOptionsE+0xb)[0x8dd43b]
node(_ZN4node16ContextifyScript3NewERKN2v820FunctionCallbackInfoINS1_5ValueEEE+0x5b4)[0xe0b614]
node(_ZN2v88internal25FunctionCallbackArguments4CallEPFvRKNS_20FunctionCallbackInfoINS_5ValueEEEE+0x92)[0x904ea2]
node[0x92e6b2]
[0x2a494db0963b]

Any expected version that a patch will land in?

This is concerning to me since it is blocking my upgrade to 4.3.0, which is recommended as a security upgrade.

[Trott]

@andyburke I totally understand if you want to stick with LTS release and not stable release, but if that doesn't make a difference to you, I believe this is fixed is in 5.4.0 and up.

[thefourtheye]

Is this the only place env could be null?

[jasnell]

@andyburke ... since 4.3.0 was a security release all other commits were bumped to the next round. We'll be preparing another LTS update in the coming week or two. We'll try to get this into that batch.

[andyburke]

@Trott thanks, but yes, would like to stick with LTS release.

@jasnell hmm, I am trying to upgrade from 4.2.3, so was this issue somehow introduced in the interim? I understand bumping the commits and am glad this will land relatively soon, but shouldn't this have been pretty high priority as a bug/regression?

[Trott]

@andyburke This bug exists in 4.2.3 (and every other 4.x version of Node).

$ node -v
v4.2.3
$ node test/parallel/test-debug-no-context.js assert.js:89
throw new assert.AssertionError({
^
AssertionError: null === 0
at ChildProcess.<anonymous> (/Users/trott/io.js/test/parallel/test-debug-no-context.js:15:10)
at ChildProcess.<anonymous> (/Users/trott/io.js/test/common.js:395:15)
at emitTwo (events.js:87:13)
at ChildProcess.emit (events.js:172:7)
at Process.ChildProcess._handle.onexit (internal/child_process.js:200:12)
$ 

[andyburke]

Somehow I am not seeing this in 4.2.3. Rolling my docker container back to 4.2.3 alleviates this issue. Perhaps there is some module interaction that is different, but rolling back fixed this for me.

[jasnell]

Interesting. Ok. I can understand the frustration @andyburke. The original plan had been to get a regular 4.x update out this week but the security release came up and pushed the schedule off just a bit. I'm going to be spinning up work on the next 4.x maintenance release (which is actually going to be quite large I'm afraid, there are quite a few stacked up commits) later on this week. We're going to have a Release Candidate cycle on this one so once the first RC is available let's see if the issue is resolved for you.