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

fix(net/tls) we need to call end when we got FIN if allowHalfOpen is false #13212

Merged
merged 7 commits into from
Aug 13, 2024

Conversation

cirospaciari
Copy link
Collaborator

@cirospaciari cirospaciari commented Aug 9, 2024

What does this PR do?

Fix: #13184
Fix: #11414

  • Documentation or TypeScript types (it's okay to leave the rest blank in this case)
  • Code changes

How did you verify your code works?

Added a test

@cirospaciari cirospaciari marked this pull request as draft August 10, 2024 01:29
@cirospaciari cirospaciari marked this pull request as ready for review August 10, 2024 01:29
src/js/node/net.ts Outdated Show resolved Hide resolved
@Jarred-Sumner Jarred-Sumner merged commit 460d6ed into main Aug 13, 2024
45 checks passed
@Jarred-Sumner Jarred-Sumner deleted the ciro/socket-fin branch August 13, 2024 07:24
@wellwelwel
Copy link

wellwelwel commented Aug 19, 2024

Hi @cirospaciari, thanks for this fix 🤝

I have this simple helper:

import { createConnection } from 'net';

const checkPort = (port: number, host: string): Promise<boolean> =>
  new Promise((resolve) => {
    const client = createConnection(port, host);

    client.on('connect', () => {
      client.destroy(); // it worked normally before this PR
      resolve(true);
    });

    client.on('error', () => {
      resolve(false);
    });
  });

After this PR, the process doesn't finish after a successful port check when using .destroy().

  • If I change it to client.end() instead of client.destroy(), the behavior works as expected.

I also checked the regression changes, but they don't seem to "visit" the destroy method.


Do you think it would be interesting to open an issue for this? 🙋🏻‍♂️

Note

For some reason, this behavior only occurs in GitHub Actions environment.

  • GitHub Actions:
    • ubuntu-latest
  • Bun:
    • 1.1.23+4304368fc
    • 1.1.24+85a329911
    • 1.1.25-canary.36+d55b5cc16

@cirospaciari
Copy link
Collaborator Author

import { createConnection } from 'net';

const checkPort = (port: number, host: string): Promise<boolean> =>
  new Promise((resolve) => {
    const client = createConnection(port, host);

    client.on('connect', () => {
      client.destroy(); // it worked normally before this PR
      resolve(true);
    });

    client.on('error', () => {
      resolve(false);
    });
  });

Thanks for reporting and giving the reproduction, I just opened a PR with a fix for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bun does not exit if net.Socket end (FIN) from the other side Bun does not end after closing mysql2 pool
4 participants