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

Port listening doesn't stop when using CTRL+C #781

Closed
ziimakc opened this issue Apr 16, 2024 · 11 comments · Fixed by #782
Closed

Port listening doesn't stop when using CTRL+C #781

ziimakc opened this issue Apr 16, 2024 · 11 comments · Fixed by #782

Comments

@ziimakc
Copy link

ziimakc commented Apr 16, 2024

Expected Behavior

Running and stopping server with CTRL+C should stop server from listening on port.

Actual Behavior

Running and stopping server with CTRL+C doesn't stop server from listening on port.

Steps to Reproduce the Problem

Running this command node ./server.js manually 2 times works fine, doing the same using zx ./scripts/dev.js fails second time with error about port which server is listening is already in use.

This started to happen after update to v8.

#!/usr/bin/env bash

import { $ } from "zx";

await $({ verbose: true})`node ./server.js`;

Does anything changed and I need somehow manually kill a process?

Specifications

  • Version: 8.0.1
  • Platform: wsl ubuntu 22.04
@antongolub
Copy link
Collaborator

Ouch, we should make detached option configuralble.
https://nodejs.org/api/child_process.html#optionsdetached

antongolub added a commit to antongolub/zx that referenced this issue Apr 17, 2024
@ziimakc
Copy link
Author

ziimakc commented Apr 17, 2024

@antongolub my platform is linux, will it work fine by default or do I need to specify detached=? Zx version 7 works fine

@antongolub
Copy link
Collaborator

Well, the only purpose for setting detached=true by default is to make $`sleep 999` abortable.
nodejs/node#37518

@antonmedv, what do you think?

@antonmedv
Copy link
Collaborator

I did not get this. We want to make detached=true by default?

@antongolub
Copy link
Collaborator

antongolub commented Apr 17, 2024

v7 uses false
v8 uses true to handle abort controller's corner case.
@antonmedv, Is this a some kind of sensible defaults? should we rollback the value?

@antonmedv
Copy link
Collaborator

Yes, I don't think we should detach process.

to handle abort controller's corner case.

What corner case?

@antongolub
Copy link
Collaborator

antongolub commented Apr 18, 2024

corner case?

Technically, detached + abort() kills the starter process (bash) only.

@lorisleiva
Copy link

Hi there,

I've got the same issue. What do I need to do in v8 to make CTRL+C kill the child process?

antonmedv pushed a commit that referenced this issue Apr 18, 2024
* feat: let detached opt be configurable

closes #781

* fix: set `datached` opt to false by default
@antongolub
Copy link
Collaborator

antongolub commented Apr 18, 2024

@lorisleiva, @ziimakc

Could you check the fix proposal?

@lorisleiva
Copy link

Works like a charm on my side. Thank you! 🍺

@ziimakc
Copy link
Author

ziimakc commented Apr 18, 2024

@antonmedv same, works fine, thanks!

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 a pull request may close this issue.

4 participants