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

Studio: Reclaim unavailable port after site deletion #195

Merged
merged 7 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@
"archiver": "^6.0.1",
"compressible": "2.0.18",
"compression": "1.7.4",
"cross-port-killer": "^1.4.0",
"date-fns": "^3.3.1",
"electron-squirrel-startup": "^1.0.0",
"express": "4.19.2",
Expand Down
21 changes: 21 additions & 0 deletions src/lib/port-finder.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import http from 'http';
import killPort from 'cross-port-killer';
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before deciding to use this library, I tried a few things to try to release a CLOSED port for use, including:

  • Used http.createServer to attempt to .close the port explicitly. This led to the port remaining in a CLOSED state, but not being released, which is the same as what's already happening when a site is deleted (reference).
  • Experimented with explicitly setting socket.setReuseAddress(true), so as to be able to re-use the port more quickly and also detecting/destroying open sockets.

The only success I had was with manually killing the PIDs associated with a given port using a lsof command, which is only available for Unix systems. The cross-port-killer library works for all platforms and using it felt more efficient than the code that would be required.

I'm open to further thoughts or ideas here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for explaining the reasoning behind using the library for that. It seems that the library is a wrapper and uses tools available in win32, linux, and darwin to kill the port, so it would go well with our support for Mac, Windows, and Linux in the future.


const DEFAULT_PORT = 8881;

Expand Down Expand Up @@ -72,6 +73,26 @@ class PortFinder {
this.#unavailablePorts.push( port );
}
}

public reclaimUnavailablePort( port?: number ): void {
if ( port && this.#unavailablePorts.includes( port ) ) {
killPort( port )
.then( () => {
console.log( `Killed processes using port ${ port }` );
} )
.catch( ( err ) => {
console.error( `Failed to kill processes using port ${ port }: ${ err.message }` );
} )
.finally( () => {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These steps are added to the finally block as there are some instances where we can have the desired outcome even if killPort fails. For example:

  • User deletes site, but an error occurs when attempting to kill the port.
  • The user's laptop gets put to sleep, thus causing the port to be killed by the OS.
  • User returns to laptop and creates another site in the app.
  • The reclaimed port is used thanks to the fact it was removed from the #unavailablePorts list and #searchPort was reset to the default.

// Ensure port finder cycles through newly reclaimed ports by removing
// from unavailable ports list and resetting #searchPort.
this.#unavailablePorts = this.#unavailablePorts.filter(
( unavailablePort ) => unavailablePort !== port
);
this.#searchPort = DEFAULT_PORT;
} );
}
}
}

export const portFinder = PortFinder.getInstance();
1 change: 1 addition & 0 deletions src/site-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export class SiteServer {
}
await this.stop();
servers.delete( this.details.id );
portFinder.reclaimUnavailablePort( this.details.port );
}

async start() {
Expand Down
Loading