Server start (next start) uses HTTP to communicate between threads

[davidbarratt]

Verify canary release

• I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 22.5.0: Thu Jun  8 22:22:20 PDT 2023; root:xnu-8796.121.3~7/RELEASE_ARM64_T6000
Binaries:
  Node: 18.16.1
  npm: 9.5.1
  Yarn: 1.22.19
  pnpm: N/A
Relevant packages:
  next: 13.4.7
  eslint-config-next: 13.4.7
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.1.6

Which area(s) of Next.js are affected? (leave empty if unsure)

App Router, CLI (create-next-app), Data fetching (gS(S)P, getInitialProps), Middleware / Edge (API routes, runtime)

Link to the code that reproduces this issue or a replay of the bug

next start

To Reproduce

Run next-start and with something to instrument outbound HTTP requests (i.e. New Relic) and you should see something like this:

Describe the Bug

In fdacca8 next start started using workers in order to seperate out the rendering and routing logic into seperate threads for the purposes of isolation. This seems sensible, however, the the mechanism chosen to message between the threads is.... HTTP. While that is a wonderful protocol to message between applications/processes, it incures a significant amount of overhead that is completely unnecessary to communicate between threads of the same process.

Expected Behavior

I expected Next.js to listen to HTTP requests on the single, specified port and use worker.postMessage() to forward a request to a specific worker and then listen for a response with the message event. The main thread will need to keep track of it's sent requests so it can match them up with respones (which may come back in any order). Alternatively, it's probably accaptable to create a new thread for each incoming request, which might be better anyways for handling the high CPU usage of SSR.

If the goal of this code is isolation rather than multithreading, it might be better to use a Node.js VM in which you could pass an event emitter or even the promise resolve callback. This is how Cloudflare isolates Workers from each other.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

[davidbarratt]

Ping @ijjk

[colonder]

This has been resolved with next Next.js versions, if you upgrade to the newest one, it works with specifying just one port

[davidbarratt]

@colonder which change fixed it?

[colonder]

@davidbarratt When I said "upgrade to the newest one" I meant that previously I used 13.4.6 and now I upgraded to 13.4.19 so I can't really tell which change precisely fixed it. I can check which version is the first one that started working as expected.

[colonder]

@davidbarratt I found that 13.4.13 is the first stable version that works as expected, anything before that has this issue with an additional random port. That doesn't mean that everything is cool though - some hostings (like cPanel with Phusion Passenger) still report that there have been multiple calls to server.listen() and some kind of proxy is still required. That is still an improvement though since it's much easier to work around having to proxy just one port instead of an additional random one. For anyone coming across this issue, I'm posting below how I implemented such proxy with node-http-proxy (for specific use with Phusion Passenger in this case)

if (typeof(PhusionPassenger) !== 'undefined') {
    PhusionPassenger.configure({ autoInstall: false });
}

const path = require("path");
const httpProxy = require("http-proxy")
const nextPath = path.join(__dirname, "node_modules", ".bin", "next");

// https://www.phusionpassenger.com/library/indepth/nodejs/reverse_port_binding.html
const port = process.env.NEXT_PORT || 3000;
process.env.PORT = port;
// Create proxy server
// MODIFIED!!!
httpProxy.createProxyServer({forward: `http://localhost:${port}`}).listen(typeof(PhusionPassenger) !== 'undefined' ? "passenger": 3000)

// create target server
process.argv.length = 1;
process.argv.push(nextPath, "start");
require(nextPath);

[github-actions]

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.