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

Windows IRPStackSize increased to pass test-debug-port-from-cmdline.js #2094

Closed
rvagg opened this issue Jul 2, 2015 · 17 comments
Closed

Windows IRPStackSize increased to pass test-debug-port-from-cmdline.js #2094

rvagg opened this issue Jul 2, 2015 · 17 comments
Labels
test Issues and PRs related to the tests. windows Issues and PRs related to the Windows platform.

Comments

@rvagg
Copy link
Member

rvagg commented Jul 2, 2015

@nodejs/platform-windows

test-debug-port-from-cmdline.js has been failing repeatedly on 2008 like so:

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)

So I followed the instructions @ https://support.microsoft.com/en-us/kb/106167 and increased IRPStackSize to 9 and now it appears to be passing.

This doesn't seem like an appropriate course of action because we can't advise developers to do this in order to get the test suite to pass. But since I have no idea why this is even required I'm not sure of a better way forward or how to suggest a possible fix.

@benjamingr
Copy link
Member

Isn't 9 well below the default value?

On Thu, Jul 2, 2015 at 11:55 AM, Rod Vagg notifications@github.com wrote:

@nodejs/platform-windows
https://github.com/orgs/nodejs/teams/platform-windows

test-debug-port-from-cmdline.js has been failing repeatedly on 2008 like
so:

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)

So I followed the instructions @
https://support.microsoft.com/en-us/kb/106167 and increased IRPStackSize
to 9 and now it appears to be passing.

This doesn't seem like an appropriate course of action because we can't
advise developers to do this in order to get the test suite to pass. But
since I have no idea why this is even required I'm not sure of a better way
forward or how to suggest a possible fix.


Reply to this email directly or view it on GitHub
#2094.

@rvagg
Copy link
Member Author

rvagg commented Jul 2, 2015

I believe 4 is the default when the entry doesn't exist

@benjamingr
Copy link
Member

The default value of the IRPStackSize parameter is 15. The range is from 11 (0xb hexadecimal) through 50 (0x32 hexadecimal).

https://support.microsoft.com/en-us/kb/285089

@rvagg
Copy link
Member Author

rvagg commented Jul 2, 2015

I see your KB article and raise you mine:

Note Values may range from 0x1 to 0xC. These values are equivalent to 1 to 12 in decimal notation.

If the IRPStackSize registry entry is not present, the computer uses a default value of 0x4.

https://support.microsoft.com/en-us/kb/106167

But then again, it has NT 3.51 at the bottom of the page!

@benjamingr
Copy link
Member

But then again, it has NT 3.51 at the bottom of the page!

Haha, yeah, it even links to a different one from NT 4, I wonder what running io.js on any of those would look like. Anyway, I'll try a fresh VM and check.

@benjamingr
Copy link
Member

Ok, opened a new Windows Server 2008R2 VM on Azure - it is not defined:

Imgur

Would it help if I cloned io.js on it and ran the tests?

@rvagg
Copy link
Member Author

rvagg commented Jul 2, 2015

Anyone running tests on windows helps! we have too few eyes at the developer level on core code & tests on windows. See https://github.com/nodejs/build/tree/master/setup/windows if you want detailed docs of how we do it.

@benjamingr
Copy link
Member

Thanks, I'm already familiar with the procedure :) Any specific branch?

@rvagg
Copy link
Member Author

rvagg commented Jul 2, 2015

master is where we're seeing it, also I've upped the 2008 servers to 24 (decimal) now

@benjamingr
Copy link
Member

Hmm, on Windows 2008 we can't install VS2013 Express since it requires a newer version of Windows. I'll try with VS2012

@rvagg
Copy link
Member Author

rvagg commented Jul 2, 2015

vs2012 isn't going to work, you need Microsoft Visual Studio Express 2013 for Windows Desktop or a prerelease of 2015, we're running on 2008R2 just like your Azure machine, perhaps it's the fact that you're trying Express that's the problem.

@mathiask88
Copy link
Contributor

There is a difference between "Express for windows" and "Express for windows desktop". The last one is compatible with Windows Server 2008 R2 SP1 (x64).

@mscdex mscdex added windows Issues and PRs related to the Windows platform. test Issues and PRs related to the tests. labels Jul 2, 2015
@orangemocha
Copy link
Contributor

You can also use Visual Studio Community Edition. It practically supersedes Express: https://www.visualstudio.com/en-us/products/visual-studio-community-vs.aspx

I just learned about IRPStackSize, but it seems an issue with the specific driver stack on the machine, not a general issue. See this post on the subject: http://forum.sysinternals.com/when-irpstacksize-can-cause-a-problem_topic22059_post116568.html#116568

@silverwind
Copy link
Contributor

Agree with @orangemocha, the stack in question here seems to be the machine's own driver stack, which is clearly out of control for us, so there's not much we can do, except maybe parsing for that particular error message.

@silverwind
Copy link
Contributor

Same test now seems to fail with an cryptic 'Access is denied', ugh

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: Access is denied.
# 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)

https://jenkins-iojs.nodesource.com/job/iojs+pr+win/79/nodes=win2008r2/tapTestReport/test.tap-136/

@rvagg
Copy link
Member Author

rvagg commented Jul 10, 2015

I'd appreciate eyes on this: #2149

rvagg added a commit to rvagg/io.js that referenced this issue Jul 10, 2015
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
joaocgreis added a commit to JaneaSystems/node that referenced this issue Jul 15, 2015
This test was failing because the spawned process was terminated before
anything could be done, by calling child.stdin.end. With this change,
the child's stdin is no longer closed. When the stdin is not a tty,
io.js waits for the whole input before starting, so the child must be
run with --interactive to process the command sent by the parent. The
child is killed explicitly by the parent before it exits.

This test was failing silently because the asserts were not called if
nothing was received from the child. This fix moves assertOutputLines to
always run on exit.

Fixes nodejs#2094
Fixes nodejs#2177
cjihrig pushed a commit that referenced this issue Jul 16, 2015
This test was failing because the spawned process was terminated before
anything could be done, by calling child.stdin.end. With this change,
the child's stdin is no longer closed. When the stdin is not a tty,
io.js waits for the whole input before starting, so the child must be
run with --interactive to process the command sent by the parent. The
child is killed explicitly by the parent before it exits.

This test was failing silently because the asserts were not called if
nothing was received from the child. This fix moves assertOutputLines to
always run on exit.

Fixes: #2177
Refs: #2094
PR-URL: #2186
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Alexis Campailla <alexis@janeasystems.com>
@cjihrig
Copy link
Contributor

cjihrig commented Jul 16, 2015

Going to close this. The problem is hopefully fixed in 2b4b600.

@cjihrig cjihrig closed this as completed Jul 16, 2015
joaocgreis added a commit to nodejs/node-v0.x-archive that referenced this issue Jul 23, 2015
This change is a backport of 2b4b600
from io.js.

Original commit message:

  This test was failing because the spawned process was terminated
  before anything could be done, by calling child.stdin.end. With this
  change, the child's stdin is no longer closed. When the stdin is not
  a tty, io.js waits for the whole input before starting, so the child
  must be run with --interactive to process the command sent by the
  parent. The child is killed explicitly by the parent before it exits.

  This test was failing silently because the asserts were not called if
  nothing was received from the child. This fix moves assertOutputLines
  to always run on exit.

  Fixes: nodejs/node#2177
  Refs: nodejs/node#2094
  PR-URL: nodejs/node#2186
  Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  Reviewed-By: Rod Vagg <rod@vagg.org>
  Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
  Reviewed-By: Alexis Campailla <alexis@janeasystems.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: #25748
jBarz pushed a commit to ibmruntimes/node that referenced this issue Nov 4, 2016
This change is a backport of 2b4b600
from io.js.

Original commit message:

  This test was failing because the spawned process was terminated
  before anything could be done, by calling child.stdin.end. With this
  change, the child's stdin is no longer closed. When the stdin is not
  a tty, io.js waits for the whole input before starting, so the child
  must be run with --interactive to process the command sent by the
  parent. The child is killed explicitly by the parent before it exits.

  This test was failing silently because the asserts were not called if
  nothing was received from the child. This fix moves assertOutputLines
  to always run on exit.

  Fixes: nodejs/node#2177
  Refs: nodejs/node#2094
  PR-URL: nodejs/node#2186
  Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  Reviewed-By: Rod Vagg <rod@vagg.org>
  Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
  Reviewed-By: Alexis Campailla <alexis@janeasystems.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#25748
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 a pull request may close this issue.

7 participants