Skip to content

fix: clean up authenticator when server startup fails - #4129

Open
0xm1nam0 wants to merge 1 commit into
MirrorNetworking:masterfrom
0xm1nam0:codex/fix-server-startup-auth-cleanup
Open

0xm1nam0 wants to merge 1 commit into
MirrorNetworking:masterfrom
0xm1nam0:codex/fix-server-startup-auth-cleanup

Conversation

@0xm1nam0

Copy link
Copy Markdown

Problem

NetworkManager.SetupServer starts the authenticator and registers OnServerAuthenticated before calling NetworkServer.Listen. If Transport.ServerStart throws (for example, when its port is already in use), NetworkServer.active remains false. NetworkManager.StopServer then returns early, leaving the Manager's authentication listener installed and the authenticator without its matching OnStopServer call.

This is still present on master at c4f3739.

Change

Roll back the Manager-owned authentication listener and stop the authenticator when Listen throws. Preserve external event listeners and rethrow the original startup exception, even if authenticator cleanup also throws. This is deliberately limited to authentication rollback; it does not claim to perform complete transport rollback.

Regression coverage

  • Server and Host startup failures leave the connection unauthenticated while external listeners still run.
  • After explicitly shutting down the failed server, a subsequent start/stop keeps the external listener and removes the Manager listener correctly.
  • An authenticator cleanup exception does not replace the original startup exception.

Validation

  • git diff --check passes.
  • Static code review completed.
  • Unity 2022.3.62f3 on Windows, EditMode filter Mirror.Tests.NetworkManagers: 18/18 passed with the patch.
  • The identical tests against unmodified upstream: 15/18 passed, with all three new regression cases failing because the stale Manager listener still authenticates the connection.
  • Validation used an isolated copy of the upstream project without Examples or Hosting. The full upstream test suite and other Unity versions were not run.

Remove NetworkManager's authentication listener and stop the authenticator when NetworkServer.Listen throws, preserving external listeners and the original startup exception.

Add regression tests for server and host startup failures, retry, and authenticator cleanup exceptions.

Validated with Unity 2022.3.62f3: all 18 NetworkManager EditMode tests pass.
The same tests against unmodified upstream fail in all three new regression cases.

Change-Id: I1febc7820d8fbc65d91a33d0c9e3420ef09bfad6

This branch has not been deployed

No deployments
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 this pull request may close these issues.

1 participant