Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Shutting down the Ganache Server v7.0.0 #2185

Closed
PeterYinusa opened this issue Jan 25, 2022 · 4 comments · Fixed by #2667
Closed

Shutting down the Ganache Server v7.0.0 #2185

PeterYinusa opened this issue Jan 25, 2022 · 4 comments · Fixed by #2667
Assignees
Milestone

Comments

@PeterYinusa
Copy link

PeterYinusa commented Jan 25, 2022

Platform: macOS Big Sur 11.6
Ganache Version: v7.0.0
Node: v14.17.0
npm: v6.14.13

Description:

Server does not shut down with Ctrl + C

Steps to reproduce:

  1. Run a test build of MetaMask by running yarn build:test
  2. In a separate terminal start Ganache by running ganache
  3. Press Ctrl + C to shutdown the server

Expected result:
Ganache Server is shut down

Actual result:
Ganache Server continues running after receiving the shutdown signal

Notes:
Video attachment

Tested the following versions in the screen capture above.

Ganache CLI v6.12.2 (ganache-core: 2.13.2)

  • Server shuts down immediately

ganache v7.0.0 (@ganache/cli: 0.1.1, @ganache/core: 0.1.1)

  • Server continues running
@haltman-at
Copy link
Contributor

Wondering: Is the issue that it doesn't shut down, or that it takes a while to do so? I'd noticed the latter problem, but somehow didn't think to file an issue about it.

@davidmurdoch
Copy link
Member

@davidmurdoch
Copy link
Member

@haltman-at It's actually both.

If a client sends a request with a Connection: Keep Alive header, and the client:

  1. keeps the connection alive for n seconds and
  2. always uses the connection before n seconds have elapsed (like polling for the block number every 20 seconds)

then the socket connection will never been closed by the client.

When MetaMask is open it checks both of those boxes and will keep us from shutting down.

Ganache doesn't currently force close long-lived keep alive HTTP sessions when the server is closed, so the Node process remains active until the socket closes on it's own... which sometimes never occurs.

We're working on fixing this from a couple of different angles and will update this issue as we do.

@jeffsmale90 jeffsmale90 self-assigned this Mar 20, 2022
jeffsmale90 added a commit that referenced this issue Mar 21, 2022
…sponses shall have the underlying connection closed #2185

* This does not yet work in all cases, as uws-js-unofficial fallback does not support .close()
* Simple test case added to ensure that after calling close and a single succcessful request, all
subsequent requests will fail.
@jeffsmale90
Copy link
Contributor

jeffsmale90 commented Mar 21, 2022

@davidmurdoch I looked into various approaches to responding to this, and pushed an interim change to 54b38e0

Once close() is called, all subsequent responses will have end(body, closeConnection: true) called. This will require a change to the .js fallback in uws-js-unofficial to support the closeConnection parameter https://github.com/trufflesuite/uws-js-unofficial/blob/786f94341a22e40e5afeff47b818066b9222ca86/src/fallback/http-response.ts#L110. It's obviously not ideal, but I cannot see a way to directly close the underlying connection outside of the context of a request/response from the client.

As mentioned in uNetworking/uWebSockets.js#663 (comment) worst case is probably 10s.

I wanted to get your feedback on the approach before I pursue this further.

jeffsmale90 added a commit that referenced this issue Mar 23, 2022
…sponses shall have the underlying connection closed #2185

* This does not yet work in all cases, as uws-js-unofficial fallback does not support .close()
* Simple test case added to ensure that after calling close and a single succcessful request, all
subsequent requests will fail.
davidmurdoch added a commit that referenced this issue Apr 1, 2022
…#2667)

fixes #2185

Co-authored-by: David Murdoch <187813+davidmurdoch@users.noreply.github.com>
Co-authored-by: Micaiah Reid <micaiahreid@gmail.com>
davidmurdoch added a commit that referenced this issue Apr 4, 2022
…#2667)

fixes #2185

Co-authored-by: David Murdoch <187813+davidmurdoch@users.noreply.github.com>
Co-authored-by: Micaiah Reid <micaiahreid@gmail.com>
MicaiahReid added a commit to domob1812/ganache that referenced this issue Apr 20, 2022
…trufflesuite#2667)

fixes trufflesuite#2185

Co-authored-by: David Murdoch <187813+davidmurdoch@users.noreply.github.com>
Co-authored-by: Micaiah Reid <micaiahreid@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants