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

[REG 16.6->16.7] TCP/TLS drops AsyncLocalStorage #40693

Closed
orgads opened this issue Nov 1, 2021 · 5 comments
Closed

[REG 16.6->16.7] TCP/TLS drops AsyncLocalStorage #40693

orgads opened this issue Nov 1, 2021 · 5 comments
Labels
async_hooks Issues and PRs related to the async hooks subsystem. confirmed-bug Issues with confirmed bugs.

Comments

@orgads
Copy link
Contributor

orgads commented Nov 1, 2021

Version

v16.13.0

Platform

Microsoft Windows NT 10.0.19043.0 x64

Subsystem

tcp/tls/async_hooks

What steps will reproduce the bug?

const { AsyncLocalStorage } = require('async_hooks');
const net = require('net');

const asyncLocalStorage = new AsyncLocalStorage();

asyncLocalStorage.run({val: 'abcd'}, () => {
  const socket = new net.Socket();
  socket.on('data', () => {
    // This is 'abcd' with Node 16.6.x, and undefined with Node >=16.7.0
    console.log(asyncLocalStorage.getStore()?.val);
  });
  socket.connect(80, 'google.com', function() {
    socket.write('GET /\n');
  });
});

TLS socket is also broken:

const { AsyncLocalStorage } = require('async_hooks');
const tls = require('tls');

const asyncLocalStorage = new AsyncLocalStorage();

asyncLocalStorage.run({val: 'abcd'}, () => {
  const socket = tls.connect({ host: 'google.com', port: 443 });
  socket.on('data', () => {
    // This is 'abcd' with Node 16.6.x, and undefined with Node >=16.7.0
    console.log(asyncLocalStorage.getStore()?.val);
    socket.end();
  });
  socket.on('error', console.error);
  socket.write('GET /\n');
});

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior?

It should write abcd.

What do you see instead?

undefined

Additional information

No response

@Mesteery Mesteery added the async_hooks Issues and PRs related to the async hooks subsystem. label Nov 1, 2021
@orgads orgads changed the title [REG 14->16] TLS drops AsyncLocalStorage [REG 14->16] TCP/TLS drops AsyncLocalStorage Nov 1, 2021
@orgads orgads changed the title [REG 14->16] TCP/TLS drops AsyncLocalStorage [REG 16.6->16.7] TCP/TLS drops AsyncLocalStorage Nov 2, 2021
@orgads
Copy link
Contributor Author

orgads commented Nov 2, 2021

Bisected with official versions. It broke between 16.6.2 and 16.7.0.

@orgads
Copy link
Contributor Author

orgads commented Nov 2, 2021

Bisected with git. Caused by #38468.

a80c989306c152e76fc03b59634303a11183e0c5 is the first bad commit
commit a80c989306c152e76fc03b59634303a11183e0c5
Author: Darshan Sen <[email protected]>
Date:   Thu Apr 29 20:47:09 2021 +0530

    async_hooks: merge resource_symbol with owner_symbol

    Signed-off-by: Darshan Sen <[email protected]>

    PR-URL: https://github.com/nodejs/node/pull/38468
    Reviewed-By: James M Snell <[email protected]>
    Reviewed-By: Anna Henningsen <[email protected]>
    Reviewed-By: Rich Trott <[email protected]>
    Reviewed-By: Benjamin Gruenbaum <[email protected]>

@targos
Copy link
Member

targos commented Nov 2, 2021

@nodejs/async_hooks

@orgads
Copy link
Contributor Author

orgads commented Nov 4, 2021

ping? @RaisinTen?

@RaisinTen
Copy link
Contributor

PR: #40741

RaisinTen added a commit to RaisinTen/node that referenced this issue Nov 13, 2021
@Flarna Flarna added the confirmed-bug Issues with confirmed bugs. label Nov 14, 2021
RaisinTen added a commit to RaisinTen/node that referenced this issue Nov 14, 2021
RaisinTen added a commit that referenced this issue Nov 18, 2021
This reverts commit 937bbc5.

PR-URL: #40741
Fixes: #40693
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
RaisinTen added a commit that referenced this issue Nov 18, 2021
This reverts commit 7ca2f13.

PR-URL: #40741
Fixes: #40693
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
targos pushed a commit that referenced this issue Nov 21, 2021
Fixes: #40693

Signed-off-by: Darshan Sen <[email protected]>

PR-URL: #40741
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
targos pushed a commit that referenced this issue Nov 21, 2021
This reverts commit 937bbc5.

PR-URL: #40741
Fixes: #40693
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
targos pushed a commit that referenced this issue Nov 21, 2021
This reverts commit 7ca2f13.

PR-URL: #40741
Fixes: #40693
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
danielleadams pushed a commit that referenced this issue Jan 30, 2022
Fixes: #40693

Signed-off-by: Darshan Sen <[email protected]>

PR-URL: #40741
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
danielleadams pushed a commit that referenced this issue Jan 30, 2022
This reverts commit 937bbc5.

PR-URL: #40741
Fixes: #40693
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
danielleadams pushed a commit that referenced this issue Jan 30, 2022
This reverts commit 7ca2f13.

PR-URL: #40741
Fixes: #40693
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
danielleadams pushed a commit that referenced this issue Feb 1, 2022
Fixes: #40693

Signed-off-by: Darshan Sen <[email protected]>

PR-URL: #40741
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
danielleadams pushed a commit that referenced this issue Feb 1, 2022
This reverts commit 937bbc5.

PR-URL: #40741
Fixes: #40693
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
danielleadams pushed a commit that referenced this issue Feb 1, 2022
This reverts commit 7ca2f13.

PR-URL: #40741
Fixes: #40693
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. confirmed-bug Issues with confirmed bugs.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants