Repository navigation
parallel/test-tls-server-verify is awfully slow on Windows #1461
Description
Activity
I think the cause for the long runtime is in OpenSSL. The
s_clienton windows generates extra entropy by loading screen memory and this takes ~1-2s on each spawn. I don't get why they are not using something like windowsCryptGenRandom()function but I'm no openssl or even crypto expert so correct me if I am wrong.@mathiask88 thanks for digging that up. Seems to have been around for a while.
/cc @indutny @piscisaureus on using
CryptGetRandom()@mathiask88 The test has always been slow for me, but I never got any further than figuring out that the openssl command-line client is really slow. Thanks for digging this up.
@jbergstroem Yes and I think it is not necessary. The client runs
app_RAND_load_file()that returns on windows something likeC:\.rndand tries to read that entropy file. If this failsRAND_status()gets called and if this is called for the first time openssl gets initialized withRAND_poll()and that function callsCryptGetRandom()on Windows NT+ or Windows CE 3.0+ anyway. So I thinkRAND_screen()is a relict where OpenSSL didn't use an OS-based seed initialization.But I tested a bit more and it seems that the screen loading is not the bottleneck here. It doesn't take ~1s as I thought. If I run a server like in the test and then a *.bat file with
echo QUIT | openssl-cli.exe s_client -connect 127.0.0.1:12346the openssl-cli takes about 2s to exit after_(un)authed.The test is slow on Windows because of two 1 second delays in the Windows version of openssl-cli.exe. We have reported the issue to openssl: http://rt.openssl.org/Ticket/Display.html?user=guest&pass=guest&id=3849
Now working on a workaround, modifying the test to let all those test cases run in parallel instead of serialized, so that it finishes quickly in spite of the openssl-cli delay.
We will submit the change to io.js as well as Node.
We will submit the change to io.js as well as Node.
Awesome, thanks!
Do you folks mind reviewing the change here: nodejs/node-v0.x-archive#25368, to make sure we have a consistent +1? I can then cherry pick the commits to io.js in another PR. Thanks!
/cc @nodejs/crypto to @orangemocha's comment above, sounds positive to me but could do with a word of encouragement from someone in the crypto team
I looked at this and found that we can have one more improvement to use
child.kill()not to wait for child processes to be ended. shigeki@ad45f88RAND_screen() on my Windows takes more than one second. I think we need not to have a good client randomness in this test. Adding a -no-rand-screen option for openssl s_client on Windows is one of ideas to remove this overhead. shigeki@12be7ec and shigeki@dae36fb
The benchmark results are
Current TotalSeconds : 86.2152129 joyent/node#25368 TotalSeconds : 41.0233476 joyent/node#25368 + child.kill() TotalSeconds : 33.9199469 joyent/node#25368 + child.kill() + -no-rand-screen TotalSeconds : 9.53518819.5 seconds are still slow compared to Unix but it seems to be enough good for CI.
I'm all for
--no-rand-screen; this openssl binary isn't used for anything other than verifying our test suite.Regarding using child.kill(), I wonder if this changes the test in way that could limit its effectiveness in catching bugs. But I will add shigeki@ad45f88 to the PR and let people who are more experienced in this area provide feedback.
Regarding,
--no-rand-screenit sounds like a wonderful feature. But it would be a floating patch on a dependency, and I wonder if that would be appropriate here. In node we usually tend to limit those floating patches to changes that have already been made in the upstream project, or are high priority and at least have a clear path for being accepted upstream.Updated nodejs/node-v0.x-archive#25368
119 remaining items
- added a commit that references this issue
on Mar 27, 2018 - added a commit that references this issue
on Mar 28, 2018 - added a commit that references this issue
on Aug 15, 2018 - added a commit that references this issue
on Feb 26, 2019 - added a commit that references this issue
on Sep 5, 2019 - added a commit that references this issue
on Sep 19, 2019 - added 2 commits that reference this issue
on May 5, 2024 - added a commit that references this issue
on Feb 17, 2025
Running this test on one of our jenkins windows bots consistently takes more than 35 seconds, while taking 0.6s on my (os x) desktop.