Skip to content

Commit

Permalink
Fix plugin hooks called before host removal (clusterio#607)
Browse files Browse the repository at this point in the history
Due a bad order of operations when a host connection was closed the
event handler listening on "close" to remove the host from the
hostConnections mapping was added after the HostConnection's event
handler listening on "close" and invoking plugin hooks in response to
the host going offline.  This in turn causes plugins that send
broadcasts in repsonse to thos hooks to throw an error due to the
broadcast sending code assuming connections that exist are valid target
to send messages to.

Fix by ordering the listener for the removal of the host connection
before the listener invoking plugin events.
  • Loading branch information
Hornwitser committed Mar 9, 2024
1 parent a8ce8e7 commit 79c1a2d
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Many thanks to the following for contributing to this release:
### Changes

- Fixed Inventory Sync reporting a negative database size [¤598](https://github.com/clusterio/clusterio/pull/598).
- Fixed plugin events being invoked while a host connection was in an invalid state.

Many thanks to the following for contributing to this release:
[@Cooldude2606](https://github.com/Cooldude2606)
Expand Down
8 changes: 6 additions & 2 deletions packages/controller/src/WsServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,14 +284,18 @@ ${err.stack}`
);
}

connection = new HostConnection(data, connector, this.controller, this.remoteAddr(req));
// Handle close before HostConnection does. This avoids a host
// connection event happening between the connection breaking and
// it being removed from the host connections list.
connector.on("close", () => {
if (this.hostConnections.get(data.id) === connection) {
this.hostConnections.delete(data.id);
} else {
logger.warn("Unlisted HostConnection closed");
}
});

connection = new HostConnection(data, connector, this.controller, this.remoteAddr(req));
this.hostConnections.set(data.id, connection);
let src = new lib.Address(lib.Address.host, data.id);
connector.ready(socket, src, sessionToken, undefined);
Expand Down Expand Up @@ -331,10 +335,10 @@ ${err.stack}`
let [connector, sessionToken] = this.createSession(new lib.Address(lib.Address.control, id));

logger.verbose(`WsServer | registered control ${id} from ${this.remoteAddr(req)}`);
let connection = new ControlConnection(data, connector, this.controller, user, id);
connector.on("close", () => {
this.controlConnections.delete(id);
});
let connection = new ControlConnection(data, connector, this.controller, user, id);
this.controlConnections.set(id, connection);
let account = new lib.AccountDetails(
user.name,
Expand Down

0 comments on commit 79c1a2d

Please sign in to comment.