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-http-regr-gh-2928 is flaky #49564

Closed
joyeecheung opened this issue Sep 8, 2023 · 8 comments · Fixed by #50240
Closed

test-http-regr-gh-2928 is flaky #49564

joyeecheung opened this issue Sep 8, 2023 · 8 comments · Fixed by #50240
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. windows Issues and PRs related to the Windows platform.

Comments

@joyeecheung
Copy link
Member

joyeecheung commented Sep 8, 2023

Test

sequential/test-http-regr-gh-2928.js

Platform

FreeBSD, Windows

Console output

<details>
<summary><a href="https://ci.nodejs.org/job/node-test-binary-windows-js-suites/RUN_SUBSET=3,nodes=win2016-COMPILED_BY-vs2022-x86/22923/console">Example</a></summary>


not ok 933 sequential/test-http-regr-gh-2928
  ---
  duration_ms: 1267.60600
  severity: fail
  exitcode: 3221226505
  stack: |-
    .+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+....
```

Build links

test-azure_msft-win2016_vs2017-x64-2, test-digitalocean-freebsd12-x64-1, test-rackspace-win2012r2_vs2015-x64-2, test-digitalocean-freebsd12-x64-2, test-rackspace-win2012r2_vs2015-x64-1

Additional information

nodejs/reliability#658

@joyeecheung joyeecheung added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Sep 8, 2023
@github-actions github-actions bot added freebsd Issues and PRs related to the FreeBSD platform. windows Issues and PRs related to the Windows platform. labels Sep 8, 2023
@joyeecheung
Copy link
Member Author

A quick search in reliability shows that this has been flaky for over a year. This is long enough that I think we should at least mark it as flaky to avoid blocking the CI.

@lpinca
Copy link
Member

lpinca commented Sep 10, 2023

I'm investigating it here #49574.

lpinca added a commit to lpinca/node that referenced this issue Sep 10, 2023
Hard code the value of the host parameter to `common.localhostIPv4` in
`server.listen()` and `net.connect()`. This

1. ensures that the client `socket._handle` is not reinitialized during
   connection due to the family autodetection algorithm, preventing
   `parser.consume()` from being called with an invalid `socket._handle`
   parameter.
2. works around an issue in the FreeBSD 12 machine where the stress test
   is run where some sockets get stuck after connection.

Closes: nodejs#49565
Fixes: nodejs#49564
lpinca added a commit to lpinca/node that referenced this issue Sep 10, 2023
Hard code the value of the host parameter to `common.localhostIPv4` in
`server.listen()` and `net.connect()`. This

1. ensures that the client `socket._handle` is not reinitialized during
   connection due to the family autodetection algorithm, preventing
   `parser.consume()` from being called with an invalid `socket._handle`
   parameter.
2. works around an issue in the FreeBSD 12 machine where the stress test
   is run where some sockets get stuck after connection.

Closes: nodejs#49565
Fixes: nodejs#49564
@lpinca lpinca closed this as completed in 18e00a5 Sep 18, 2023
ruyadorno pushed a commit that referenced this issue Sep 28, 2023
Hard code the value of the host parameter to `common.localhostIPv4` in
`server.listen()` and `net.connect()`. This

1. ensures that the client `socket._handle` is not reinitialized during
   connection due to the family autodetection algorithm, preventing
   `parser.consume()` from being called with an invalid `socket._handle`
   parameter.
2. works around an issue in the FreeBSD 12 machine where the stress test
   is run where some sockets get stuck after connection.

PR-URL: #49574
Closes: #49565
Fixes: #49564
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@lpinca
Copy link
Member

lpinca commented Sep 30, 2023

It failed again on Windows: https://ci.nodejs.org/job/node-test-binary-windows-js-suites/23448/

not ok 936 sequential/test-http-regr-gh-2928
  ---
  duration_ms: 470.03300
  severity: fail
  exitcode: 3221226505
  stack: |-
    .+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.+.
  ...

@lpinca lpinca reopened this Sep 30, 2023
@lpinca lpinca removed the freebsd Issues and PRs related to the FreeBSD platform. label Sep 30, 2023
@anonrig
Copy link
Member

anonrig commented Oct 18, 2023

@richardlau
Copy link
Member

Flaky on Ubuntu 22.04 - https://ci.nodejs.org/job/node-test-binary-windows-js-suites/23886/

That's not an Ubuntu failure.

nodejs-github-bot pushed a commit that referenced this issue Oct 20, 2023
The maximum number of parsers in the free list is set to 1000. However
the test does not need to use this maximum. Reduce it to 50.

Refs: #50228 (comment)
PR-URL: #50240
Fixes: #49564
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
targos pushed a commit that referenced this issue Oct 23, 2023
The maximum number of parsers in the free list is set to 1000. However
the test does not need to use this maximum. Reduce it to 50.

Refs: #50228 (comment)
PR-URL: #50240
Fixes: #49564
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
alexfernandez pushed a commit to alexfernandez/node that referenced this issue Nov 1, 2023
Hard code the value of the host parameter to `common.localhostIPv4` in
`server.listen()` and `net.connect()`. This

1. ensures that the client `socket._handle` is not reinitialized during
   connection due to the family autodetection algorithm, preventing
   `parser.consume()` from being called with an invalid `socket._handle`
   parameter.
2. works around an issue in the FreeBSD 12 machine where the stress test
   is run where some sockets get stuck after connection.

PR-URL: nodejs#49574
Closes: nodejs#49565
Fixes: nodejs#49564
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: James M Snell <[email protected]>
alexfernandez pushed a commit to alexfernandez/node that referenced this issue Nov 1, 2023
The maximum number of parsers in the free list is set to 1000. However
the test does not need to use this maximum. Reduce it to 50.

Refs: nodejs#50228 (comment)
PR-URL: nodejs#50240
Fixes: nodejs#49564
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
targos pushed a commit that referenced this issue Nov 11, 2023
The maximum number of parsers in the free list is set to 1000. However
the test does not need to use this maximum. Reduce it to 50.

Refs: #50228 (comment)
PR-URL: #50240
Fixes: #49564
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
@mhdawson mhdawson reopened this Mar 25, 2024
@lpinca
Copy link
Member

lpinca commented Mar 25, 2024

The process crashed. It is not a timeout this time.

richardlau pushed a commit that referenced this issue Apr 17, 2024
Hard code the value of the host parameter to `common.localhostIPv4` in
`server.listen()` and `net.connect()`. This

1. ensures that the client `socket._handle` is not reinitialized during
   connection due to the family autodetection algorithm, preventing
   `parser.consume()` from being called with an invalid `socket._handle`
   parameter.
2. works around an issue in the FreeBSD 12 machine where the stress test
   is run where some sockets get stuck after connection.

PR-URL: #49574
Backport-PR-URL: #52384
Closes: #49565
Fixes: #49564
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: James M Snell <[email protected]>
richardlau pushed a commit that referenced this issue Apr 17, 2024
The maximum number of parsers in the free list is set to 1000. However
the test does not need to use this maximum. Reduce it to 50.

Refs: #50228 (comment)
PR-URL: #50240
Backport-PR-URL: #52384
Fixes: #49564
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this issue May 5, 2024
Hard code the value of the host parameter to `common.localhostIPv4` in
`server.listen()` and `net.connect()`. This

1. ensures that the client `socket._handle` is not reinitialized during
   connection due to the family autodetection algorithm, preventing
   `parser.consume()` from being called with an invalid `socket._handle`
   parameter.
2. works around an issue in the FreeBSD 12 machine where the stress test
   is run where some sockets get stuck after connection.

PR-URL: nodejs/node#49574
Closes: nodejs/node#49565
Fixes: nodejs/node#49564
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: James M Snell <[email protected]>
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this issue May 5, 2024
The maximum number of parsers in the free list is set to 1000. However
the test does not need to use this maximum. Reduce it to 50.

Refs: nodejs/node#50228 (comment)
PR-URL: nodejs/node#50240
Fixes: nodejs/node#49564
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
@lpinca
Copy link
Member

lpinca commented Sep 11, 2024

There have been no new failures since March, closing.

@lpinca lpinca closed this as completed Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. windows Issues and PRs related to the Windows platform.
Projects
None yet
5 participants