-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
Windows - inspector - intermittent ? - inspector/test-bindings #15558
Comments
@nodejs/platform-windows |
Looking at https://ci.nodejs.org/view/All/job/node-daily-master/ , this has happened for 3 days (since September 20). There's also `inspector/test-debug-end`.
cc @nodejs/testing and @eugeneo perhaps |
Here is another stack trace
|
@joaocgreis it is probably a good idea to open a new issue for the other failure? |
@BridgeAR they started at about the same time in the same subsystem, so they might be related. We can probably address them here until we figure out if they are related or not, but feel free to open another issue if you think that will help. |
Is anyone actively looking at this? (Saw another one today and I'm looking at CI a lot less than usual, so a good chance there's lots more.) I'm hoping we can get CI reliably green in preparation for Code + Learn next week. |
I'm trying to repro locally. |
ping @nodejs/v8-inspector Any idea what might be causing these tests to fail on Windows? It would be good to have CI back to green for Code + Learn next week.... |
@refack - are you looking into it? I inspected the test case visually and feel like the test may not be deterministic (e.g. it relies on the variables order in the inspector response). I think V8 might shuffle the variables because of reasons (JIT, etc). I'm going to try and make the test more deterministic. |
(Sorry, disregard my comment - does not seem to be the case. I still would like to take a look at this test case) |
Stress test is unable to reproduce the failure - https://ci.nodejs.org/job/node-stress-single-test/1430/nodes=win2016/console |
@eugeneo I just built a binary with VS2017 that reproduces quite often, which made me think it might be a VS2017 issue. The two jobs mentioned above were also built with VS2017:
(I can send you the binary if you need it) |
More failures today, and yeah, it's vs2017 again.
Same results in https://ci.nodejs.org/job/node-test-binary-windows/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=0/11473/console |
|
PR-URL: #15747 Refs: #15558 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
PR-URL: nodejs/node#15747 Refs: nodejs/node#15558 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
PR-URL: #15747 Refs: #15558 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
Reopening as I think the flaky tests still need to be marked non-flaky after confirming so. |
Reopening again. |
The inspector_agent depends on the context still being accessible during the destructor execution. PR-URL: nodejs/node#16472 Fixes: nodejs/node#15558 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
The inspector_agent depends on the context still being accessible during the destructor execution. PR-URL: #16472 Fixes: #15558 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
The inspector_agent depends on the context still being accessible during the destructor execution. PR-URL: #16472 Fixes: #15558 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
The inspector_agent depends on the context still being accessible during the destructor execution. PR-URL: #16472 Fixes: #15558 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
I see this issue using the JS API (e.g. there's no second thread running, libuv is not used for anything). What I am seeing is that session is receiving repeat "Debugger.paused" notifications from the V8. Number of those repetition is same as the total number of the previous sessions + 1. E.g. if I open and close 2 sessions, third session I will be notified thrice. It does not look like it is in V8 side of the Inspector too - it is a debug listener that fires the event multiple times, Inspector has a single session and properly dispatches the event there. I suspect there's something funny with some object deregistering, so could it be some destructor is now "optimized out"? I will update this bug when I have more information. |
This is now not ok 514 sequential/test-inspector-bindings # TODO : Fix flaky test
---
duration_ms: 0.240
severity: flaky
stack: |-
Expecting warning to be emitted
(node:6608) Error: We expect this
c:\workspace\node-test-binary-windows\test\common\index.js:797
(err) => process.nextTick(() => { throw err; }));
^
AssertionError [ERR_ASSERTION]: [ 'Iteration 1 variable: i expected: 1 actual: 0',
'Iteration 2 variable: i expected: 2 actual: 1',
'Iteration 2 variable: a deepStrictEqual []
at testSampleDebugSession (c:\workspace\node-test-binary-windows\test\sequential\test-inspector-bindings.js:104:10)
at doTests (c:\workspace\node-test-binary-windows\test\sequential\test-inspector-bindings.js:131:3)
at <anonymous>
at process._tickCallback (internal/process/next_tick.js:165:7)
at Function.Module.runMain (module.js:703:11)
at startup (bootstrap_node.js:195:16)
at bootstrap_node.js:646:3
... |
Still open. |
It seems like perhaps this should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that. |
Failure during run on un-related PR
https://ci.nodejs.org/job/node-test-binary-windows/11280/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=1/console
The text was updated successfully, but these errors were encountered: