fix: use Semaphore instead of Notify in OneshotTransport to prevent race condition during stateless mode

[madzarm]

Motivation and Context

Fixes issue #610

OneshotTransport uses tokio::sync::Notify to signal when the transport should close. However, Notify::notify_waiters() only wakes futures that are currently awaiting - if the notification fires before receive() starts waiting, the signal is lost forever.

This creates a race condition in serve_inner() where tokio::select! can drop the receive() future between iterations:

1. receive() returns the initial message
2. send() completes with Response/Error, calls notify_waiters()
3. select! picks join_next() branch, drops the receive() future
4. Next iteration: receive() creates NEW notified() future
5. This future waits forever - the notification already fired
6. Serve loop hangs, stream never closes

In stateless HTTP mode (Lambda, Cloud Functions), this causes requests to hang until timeout even though the tool executed successfully.

Solution

Replace Notify with Semaphore. Unlike notifications, permits persist until acquired:

• add_permits(1) stores the permit even if no one is waiting
• acquire().await returns immediately if a permit exists, or waits if not
• Dropping an acquire() future doesn't consume the permit (cancel-safe)

From tokio docs:

"This method is cancel safe. If acquire is used as the event in a tokio::select! statement and some other branch completes first, then no permits are acquired."

How Has This Been Tested?

• Deployed to AWS Lambda with API Gateway in stateless mode
• Load tested with concurrent requests during cold starts
• Specifically tested error paths (downstream 404s) which previously triggered the race ~3-5% of the time
• After fix: zero hangs across thousands of requests

Breaking Changes

None. This is an internal implementation change. The OneshotTransport public API is unchanged.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have added or updated documentation as needed

Additional context

The race window is small but hits reliably under load. In our production environment, ~3-5% of cold start requests would hang, always on error responses (possibly due to faster code paths when is_error: true).

The fix is minimal - only changes the synchronization primitive from Notify to Semaphore with equivalent semantics but proper cancellation safety.

[alexhancock]

Nice catch and fix, thank you

[madzarm]

Hi @alexhancock thanks for the quick response! I've shortened the commit message to fix the commit lint failure. I'd appreciate it if you could approve the workflow, thank you!

[madzarm]

Motivation and Context

Fixes issue #610

OneshotTransport uses tokio::sync::Notify to signal when the transport should close. However, Notify::notify_waiters() only wakes futures that are currently awaiting - if the notification fires before receive() starts waiting, the signal is lost forever.

This creates a race condition in serve_inner() where tokio::select! can drop the receive() future between iterations:

1. receive() returns the initial message
2. send() completes with Response/Error, calls notify_waiters()
3. select! picks join_next() branch, drops the receive() future
4. Next iteration: receive() creates NEW notified() future
5. This future waits forever - the notification already fired
6. Serve loop hangs, stream never closes

In stateless HTTP mode (Lambda, Cloud Functions), this causes requests to hang until timeout even though the tool executed successfully.

Solution

Replace Notify with Semaphore. Unlike notifications, permits persist until acquired:

• add_permits(1) stores the permit even if no one is waiting
• acquire().await returns immediately if a permit exists, or waits if not
• Dropping an acquire() future doesn't consume the permit (cancel-safe)

From tokio docs:

"This method is cancel safe. If acquire is used as the event in a tokio::select! statement and some other branch completes first, then no permits are acquired."

How Has This Been Tested?

• Deployed to AWS Lambda with API Gateway in stateless mode
• Load tested with concurrent requests during cold starts
• Specifically tested error paths (downstream 404s) which previously triggered the race ~3-5% of the time
• After fix: zero hangs across thousands of requests

Breaking Changes

None. This is an internal implementation change. The OneshotTransport public API is unchanged.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have added or updated documentation as needed

Additional context

The race window is small but hits reliably under load. In our production environment, ~3-5% of cold start requests would hang, always on error responses (possibly due to faster code paths when is_error: true).

The fix is minimal - only changes the synchronization primitive from Notify to Semaphore with equivalent semantics but proper cancellation safety.

[alexhancock]

Nice catch and fix, thank you

[madzarm]

Hi @alexhancock thanks for the quick response! I've shortened the commit message to fix the commit lint failure. I'd appreciate it if you could approve the workflow, thank you!

[madzarm]

Motivation and Context

Fixes issue #610

OneshotTransport uses tokio::sync::Notify to signal when the transport should close. However, Notify::notify_waiters() only wakes futures that are currently awaiting - if the notification fires before receive() starts waiting, the signal is lost forever.

This creates a race condition in serve_inner() where tokio::select! can drop the receive() future between iterations:

1. receive() returns the initial message
2. send() completes with Response/Error, calls notify_waiters()
3. select! picks join_next() branch, drops the receive() future
4. Next iteration: receive() creates NEW notified() future
5. This future waits forever - the notification already fired
6. Serve loop hangs, stream never closes

In stateless HTTP mode (Lambda, Cloud Functions), this causes requests to hang until timeout even though the tool executed successfully.

Solution

Replace Notify with Semaphore. Unlike notifications, permits persist until acquired:

• add_permits(1) stores the permit even if no one is waiting
• acquire().await returns immediately if a permit exists, or waits if not
• Dropping an acquire() future doesn't consume the permit (cancel-safe)

From tokio docs:

"This method is cancel safe. If acquire is used as the event in a tokio::select! statement and some other branch completes first, then no permits are acquired."

How Has This Been Tested?

• Deployed to AWS Lambda with API Gateway in stateless mode
• Load tested with concurrent requests during cold starts
• Specifically tested error paths (downstream 404s) which previously triggered the race ~3-5% of the time
• After fix: zero hangs across thousands of requests

Breaking Changes

None. This is an internal implementation change. The OneshotTransport public API is unchanged.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have added or updated documentation as needed

Additional context

The race window is small but hits reliably under load. In our production environment, ~3-5% of cold start requests would hang, always on error responses (possibly due to faster code paths when is_error: true).

The fix is minimal - only changes the synchronization primitive from Notify to Semaphore with equivalent semantics but proper cancellation safety.

[alexhancock]

Nice catch and fix, thank you

[madzarm]

Hi @alexhancock thanks for the quick response! I've shortened the commit message to fix the commit lint failure. I'd appreciate it if you could approve the workflow, thank you!

[madzarm]

Motivation and Context

Fixes issue #610

OneshotTransport uses tokio::sync::Notify to signal when the transport should close. However, Notify::notify_waiters() only wakes futures that are currently awaiting - if the notification fires before receive() starts waiting, the signal is lost forever.

This creates a race condition in serve_inner() where tokio::select! can drop the receive() future between iterations:

1. receive() returns the initial message
2. send() completes with Response/Error, calls notify_waiters()
3. select! picks join_next() branch, drops the receive() future
4. Next iteration: receive() creates NEW notified() future
5. This future waits forever - the notification already fired
6. Serve loop hangs, stream never closes

In stateless HTTP mode (Lambda, Cloud Functions), this causes requests to hang until timeout even though the tool executed successfully.

Solution

Replace Notify with Semaphore. Unlike notifications, permits persist until acquired:

• add_permits(1) stores the permit even if no one is waiting
• acquire().await returns immediately if a permit exists, or waits if not
• Dropping an acquire() future doesn't consume the permit (cancel-safe)

From tokio docs:

"This method is cancel safe. If acquire is used as the event in a tokio::select! statement and some other branch completes first, then no permits are acquired."

How Has This Been Tested?

• Deployed to AWS Lambda with API Gateway in stateless mode
• Load tested with concurrent requests during cold starts
• Specifically tested error paths (downstream 404s) which previously triggered the race ~3-5% of the time
• After fix: zero hangs across thousands of requests

Breaking Changes

None. This is an internal implementation change. The OneshotTransport public API is unchanged.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have added or updated documentation as needed

Additional context

The race window is small but hits reliably under load. In our production environment, ~3-5% of cold start requests would hang, always on error responses (possibly due to faster code paths when is_error: true).

The fix is minimal - only changes the synchronization primitive from Notify to Semaphore with equivalent semantics but proper cancellation safety.

[alexhancock]

Nice catch and fix, thank you

[madzarm]

Hi @alexhancock thanks for the quick response! I've shortened the commit message to fix the commit lint failure. I'd appreciate it if you could approve the workflow, thank you!

[madzarm]

Motivation and Context

Fixes issue #610

OneshotTransport uses tokio::sync::Notify to signal when the transport should close. However, Notify::notify_waiters() only wakes futures that are currently awaiting - if the notification fires before receive() starts waiting, the signal is lost forever.

This creates a race condition in serve_inner() where tokio::select! can drop the receive() future between iterations:

1. receive() returns the initial message
2. send() completes with Response/Error, calls notify_waiters()
3. select! picks join_next() branch, drops the receive() future
4. Next iteration: receive() creates NEW notified() future
5. This future waits forever - the notification already fired
6. Serve loop hangs, stream never closes

In stateless HTTP mode (Lambda, Cloud Functions), this causes requests to hang until timeout even though the tool executed successfully.

Solution

Replace Notify with Semaphore. Unlike notifications, permits persist until acquired:

• add_permits(1) stores the permit even if no one is waiting
• acquire().await returns immediately if a permit exists, or waits if not
• Dropping an acquire() future doesn't consume the permit (cancel-safe)

From tokio docs:

"This method is cancel safe. If acquire is used as the event in a tokio::select! statement and some other branch completes first, then no permits are acquired."

How Has This Been Tested?

• Deployed to AWS Lambda with API Gateway in stateless mode
• Load tested with concurrent requests during cold starts
• Specifically tested error paths (downstream 404s) which previously triggered the race ~3-5% of the time
• After fix: zero hangs across thousands of requests

Breaking Changes

None. This is an internal implementation change. The OneshotTransport public API is unchanged.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have added or updated documentation as needed

Additional context

The race window is small but hits reliably under load. In our production environment, ~3-5% of cold start requests would hang, always on error responses (possibly due to faster code paths when is_error: true).

The fix is minimal - only changes the synchronization primitive from Notify to Semaphore with equivalent semantics but proper cancellation safety.

[alexhancock]

Nice catch and fix, thank you

[madzarm]

Hi @alexhancock thanks for the quick response! I've shortened the commit message to fix the commit lint failure. I'd appreciate it if you could approve the workflow, thank you!

[madzarm]

Motivation and Context

Fixes issue #610

OneshotTransport uses tokio::sync::Notify to signal when the transport should close. However, Notify::notify_waiters() only wakes futures that are currently awaiting - if the notification fires before receive() starts waiting, the signal is lost forever.

This creates a race condition in serve_inner() where tokio::select! can drop the receive() future between iterations:

1. receive() returns the initial message
2. send() completes with Response/Error, calls notify_waiters()
3. select! picks join_next() branch, drops the receive() future
4. Next iteration: receive() creates NEW notified() future
5. This future waits forever - the notification already fired
6. Serve loop hangs, stream never closes

In stateless HTTP mode (Lambda, Cloud Functions), this causes requests to hang until timeout even though the tool executed successfully.

Solution

Replace Notify with Semaphore. Unlike notifications, permits persist until acquired:

• add_permits(1) stores the permit even if no one is waiting
• acquire().await returns immediately if a permit exists, or waits if not
• Dropping an acquire() future doesn't consume the permit (cancel-safe)

From tokio docs:

"This method is cancel safe. If acquire is used as the event in a tokio::select! statement and some other branch completes first, then no permits are acquired."

How Has This Been Tested?

• Deployed to AWS Lambda with API Gateway in stateless mode
• Load tested with concurrent requests during cold starts
• Specifically tested error paths (downstream 404s) which previously triggered the race ~3-5% of the time
• After fix: zero hangs across thousands of requests

Breaking Changes

None. This is an internal implementation change. The OneshotTransport public API is unchanged.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have added or updated documentation as needed

Additional context

The race window is small but hits reliably under load. In our production environment, ~3-5% of cold start requests would hang, always on error responses (possibly due to faster code paths when is_error: true).

The fix is minimal - only changes the synchronization primitive from Notify to Semaphore with equivalent semantics but proper cancellation safety.

[alexhancock]

Nice catch and fix, thank you

[madzarm]

Hi @alexhancock thanks for the quick response! I've shortened the commit message to fix the commit lint failure. I'd appreciate it if you could approve the workflow, thank you!

[madzarm]

Motivation and Context

Fixes issue #610

OneshotTransport uses tokio::sync::Notify to signal when the transport should close. However, Notify::notify_waiters() only wakes futures that are currently awaiting - if the notification fires before receive() starts waiting, the signal is lost forever.

This creates a race condition in serve_inner() where tokio::select! can drop the receive() future between iterations:

1. receive() returns the initial message
2. send() completes with Response/Error, calls notify_waiters()
3. select! picks join_next() branch, drops the receive() future
4. Next iteration: receive() creates NEW notified() future
5. This future waits forever - the notification already fired
6. Serve loop hangs, stream never closes

In stateless HTTP mode (Lambda, Cloud Functions), this causes requests to hang until timeout even though the tool executed successfully.

Solution

Replace Notify with Semaphore. Unlike notifications, permits persist until acquired:

• add_permits(1) stores the permit even if no one is waiting
• acquire().await returns immediately if a permit exists, or waits if not
• Dropping an acquire() future doesn't consume the permit (cancel-safe)

From tokio docs:

"This method is cancel safe. If acquire is used as the event in a tokio::select! statement and some other branch completes first, then no permits are acquired."

How Has This Been Tested?

• Deployed to AWS Lambda with API Gateway in stateless mode
• Load tested with concurrent requests during cold starts
• Specifically tested error paths (downstream 404s) which previously triggered the race ~3-5% of the time
• After fix: zero hangs across thousands of requests

Breaking Changes

None. This is an internal implementation change. The OneshotTransport public API is unchanged.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have added or updated documentation as needed

Additional context

The race window is small but hits reliably under load. In our production environment, ~3-5% of cold start requests would hang, always on error responses (possibly due to faster code paths when is_error: true).

The fix is minimal - only changes the synchronization primitive from Notify to Semaphore with equivalent semantics but proper cancellation safety.

[alexhancock]

Nice catch and fix, thank you

[madzarm]

Hi @alexhancock thanks for the quick response! I've shortened the commit message to fix the commit lint failure. I'd appreciate it if you could approve the workflow, thank you!

[madzarm]

Motivation and Context

Fixes issue #610

OneshotTransport uses tokio::sync::Notify to signal when the transport should close. However, Notify::notify_waiters() only wakes futures that are currently awaiting - if the notification fires before receive() starts waiting, the signal is lost forever.

This creates a race condition in serve_inner() where tokio::select! can drop the receive() future between iterations:

1. receive() returns the initial message
2. send() completes with Response/Error, calls notify_waiters()
3. select! picks join_next() branch, drops the receive() future
4. Next iteration: receive() creates NEW notified() future
5. This future waits forever - the notification already fired
6. Serve loop hangs, stream never closes

In stateless HTTP mode (Lambda, Cloud Functions), this causes requests to hang until timeout even though the tool executed successfully.

Solution

Replace Notify with Semaphore. Unlike notifications, permits persist until acquired:

• add_permits(1) stores the permit even if no one is waiting
• acquire().await returns immediately if a permit exists, or waits if not
• Dropping an acquire() future doesn't consume the permit (cancel-safe)

From tokio docs:

"This method is cancel safe. If acquire is used as the event in a tokio::select! statement and some other branch completes first, then no permits are acquired."

How Has This Been Tested?

• Deployed to AWS Lambda with API Gateway in stateless mode
• Load tested with concurrent requests during cold starts
• Specifically tested error paths (downstream 404s) which previously triggered the race ~3-5% of the time
• After fix: zero hangs across thousands of requests

Breaking Changes

None. This is an internal implementation change. The OneshotTransport public API is unchanged.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have added or updated documentation as needed

Additional context

The race window is small but hits reliably under load. In our production environment, ~3-5% of cold start requests would hang, always on error responses (possibly due to faster code paths when is_error: true).

The fix is minimal - only changes the synchronization primitive from Notify to Semaphore with equivalent semantics but proper cancellation safety.

[alexhancock]

Nice catch and fix, thank you

[madzarm]

Hi @alexhancock thanks for the quick response! I've shortened the commit message to fix the commit lint failure. I'd appreciate it if you could approve the workflow, thank you!