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

test: ignore IRPStackSize bug on win32 #2149

Closed
wants to merge 1 commit into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Jul 10, 2015

Some driver stacks on win32 will trigger an occasional error on test-debug-port-from-cmdline.js:

not ok 136 - test-debug-port-from-cmdline.js
#c:\workspace\iojs+pr+win\nodes\win2008r2\test\parallel\test-debug-port-from-cmdline.js:20
#    process._debugProcess(child.pid);
#            ^
#Error: Not enough storage is available to process this command.
#    at Error (native)
#    at ChildProcess.onChildMsg (c:\workspace\iojs+pr+win\nodes\win2008r2\test\parallel\test-debug-port-from-cmdline.js:20:13)
#    at emitTwo (events.js:87:13)
#    at ChildProcess.emit (events.js:172:7)
#    at handleMessage (internal/child_process.js:632:10)
#    at Pipe.channel.onread (internal/child_process.js:421:11)

My attempt at a workaround is to check for that error and ignore the test entirely. But unfortunately it's not so straightforward because now the test is failing with:

not ok 136 - test-debug-port-from-cmdline.js
#c:\workspace\iojs+pr+win\nodes\win2008r2\test\parallel\test-debug-port-from-cmdline.js:29
#      throw e;
#            ^
#Error: Access is denied.
#    at Error (native)
#    at ChildProcess.onChildMsg (c:\workspace\iojs+pr+win\nodes\win2008r2\test\parallel\test-debug-port-from-cmdline.js:21:15)
#    at emitTwo (events.js:87:13)
#    at ChildProcess.emit (events.js:172:7)
#    at handleMessage (internal/child_process.js:632:10)
#    at Pipe.channel.onread (internal/child_process.js:421:11)

Can anyone with background or any kind of clue on this suggest a better workaround or do I just need to catch the error and ignore if win32 (I'd rather not be so blanket about it).

@cjihrig
Copy link
Contributor

cjihrig commented Jul 10, 2015

Is this specific to Windows? I saw this recently on OS X - https://jenkins-iojs.nodesource.com/job/iojs+pr+osx/105/nodes=osx1010/tapTestReport/test.tap-136/

try {
process._debugProcess(child.pid);
} catch (e) {
if (process.platform === 'win32' && (e.message ===
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use common.isWindows to check for Windows.

@mscdex mscdex added windows Issues and PRs related to the Windows platform. test Issues and PRs related to the tests. labels Jul 10, 2015
@rvagg
Copy link
Member Author

rvagg commented Jul 10, 2015

ouch .. I was sure this was Windows only, and our Windows 2008 machines in particular. It could be a different failure given that there is no output along with that error and it's actually failing from a timeout (console output shows ~60 seconds for that one).

Some driver stacks on win32 will trigger an occasional error on
test-debug-port-from-cmdline.js, reporting:
  Not enough storage is available to process this command
Workaround is to check for that error and ignore the test entirely.

Closes: nodejs#2094
@rvagg rvagg force-pushed the irpstacksize-windows-ignore branch from 02c6988 to c03e057 Compare July 10, 2015 01:53
'Not enough storage is available to process this command.' ||
e.message === 'Access is denied.')) {
child.kill();
console.log('Encountered IRPStackSize bug on win32, ignoring test');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe comment linking to this issue?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to follow the convention

1..0 # Skipped: Encountered IRPStackSize bug on win32, ignoring test

while skipping tests, so that TAP Plugin can pick them.

@orangemocha
Copy link
Contributor

@rvagg if the issue is as hypothesized, a buggy driver setup, I am not too convinced that changing the test is the best course of action. Shouldn't we rather investigate and fix that machine configuration?

@silverwind
Copy link
Contributor

@cjihrig might be a different issue. Your OS X case times out after 60s, Windows throws fast.

I think we should at least try to investigate what process._debugProcess does that could cause weird errors like this.

@rvagg
Copy link
Member Author

rvagg commented Jul 11, 2015

@orangemocha do you have time to investigate? My concern is that this is a complex issue that isn't likely to get attention any time soon and the regular spurious failures are going to be a setback for the rest of the project when they don't appear to point to real problems with node

@orangemocha
Copy link
Contributor

@rvagg , sure. Assuming this is a problem with the driver stack, the best fix I can think of is increasing IRPStackSize on the problematic machine. But I'll take a closer look. Which machines have you seen this on? All the Win2k8 jenkins slaves? Are they VMs or physical machines?

@rvagg
Copy link
Member Author

rvagg commented Jul 11, 2015

@orangemocha I fiddled briefly with IRPStackSize but it didn't seem to do much. It's been only the Windows 2008 machines. They are VMs on Rackspace.

@mathiask88
Copy link
Contributor

@silverwind AFAIK MapViewOfFile can fail with Not enough storage... e.g. if not enough contiguous(!) memory is available. And Access denied can be thrown by OpenProcess and CreateFileMapping e.g. if UAC is enabled and the process is not privileged.

@joaocgreis
Copy link
Member

@rvagg I investigated and found that the problem was in the test itself. It also failed on Linux, but silently: a correct test should always produce two lines of output.

I opened a PR: #2186

@rvagg rvagg closed this Jul 16, 2015
@rvagg rvagg deleted the irpstacksize-windows-ignore branch July 16, 2015 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants