Skip to content

fix(server): run onInitialized callbacks registered after initialization - #1019

Open
taekop wants to merge 1 commit into
modelcontextprotocol:mainfrom
taekop:fix/920-late-oninitialized
Open

taekop wants to merge 1 commit into
modelcontextprotocol:mainfrom
taekop:fix/920-late-oninitialized

Conversation

@taekop

@taekop taekop commented Sep 23, 2026

Copy link
Copy Markdown

Fixes #920

ServerSession.onInitialized put callbacks together in a plain var, and the notifications/initialized handler invoked it. That caused two problems:

  • If you register a callback after the client has sent notifications/initialized, it never runs.
  • The var is read and written without synchronization. When onInitialized is called from several threads while the handshake is in progress, two registrations can read the same old value, and one of the callbacks is then dropped.

Pending callbacks are now stored in an AtomicRef<PersistentList<() -> Unit>?>, using the same atomicfu + kotlinx-collections-immutable pattern as FeatureRegistry:

  • onInitialized adds the callback with getAndUpdate. If the ref was already null, initialization has finished, so the callback runs right away on the caller's thread.
  • The initialized handler calls getAndSet(null) and runs the list it gets back once, in registration order.

Each callback now runs at most once, and callbacks pending when notifications/initialized arrives run in registration order. (A callback never runs if the client doesn't send notifications/initialized, and if one throws, the ones after it don't run.) There is one behavior change: before, a duplicate notifications/initialized ran all callbacks again. Now it runs nothing. No public API changes (apiCheck passes).

This follows up #938, which targeted the same issue and was closed by its author.

Tests (in ServerSessionInitializeTest):

  • should run onInitialized callback registered after initialization: a callback registered after the handshake runs. Before the fix it failed with [early] instead of [early, late].
  • should run every onInitialized callback registered concurrently: 200 callbacks are registered concurrently on Dispatchers.Default before the handshake, and each must run once. Before the fix it failed in 5 out of 5 runs, with 2 to 16 callbacks dropped.
./gradlew :integration-test:jvmTest --tests "*ServerSessionInitializeTest*"   # 6 tests, 0 failures
./gradlew :kotlin-sdk-server:jvmTest                                          # 228 tests, 0 failures
./gradlew apiCheck ktlintCheck detekt                                         # pass

I haven't run the full integration-test suite (the TypeScript interop tests) or the non-JVM targets.

I used Claude Code to write this change and the PR description.

…ion (modelcontextprotocol#920)

ServerSession.onInitialized composed callbacks into a plain var, so a
callback registered after notifications/initialized was never invoked,
and concurrent registration could drop callbacks.

Keep pending callbacks in an AtomicRef<PersistentList?>. The initialized
handler swaps it to null and runs the list in registration order;
registration after that runs the callback immediately. Each callback
now runs at most once.

Fixes modelcontextprotocol#920

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.

ServerSession.onInitialized has no state replay, so late registrants silently miss the event

1 participant