Skip to content

Conversation

pankaj-bind
Copy link

Description

This update addresses a potential resource leak in the simple-auth-client example. The CallbackServer, which handles the OAuth2 callback, was not guaranteed to be shut down if an exception occurred during the connection setup.

Specifically, if an exception was raised in the connect method after callback_server.start() was called but before the main logic was entered, the finally block that called callback_server.stop() would never be reached. This would leave the HTTP server running in a background thread, consuming resources.

The Fix

The connect method in mcp_simple_auth_client/main.py has been refactored to use a single, top-level try...finally block. This ensures that callback_server.stop() is executed regardless of whether the connection process succeeds or fails with an exception.

This change makes the example client more robust and reliable by guaranteeing proper resource cleanup.

Type of change

  • 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 not work as expected)
  • This change requires a 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

@pankaj-bind pankaj-bind requested review from a team and pcarleton September 5, 2025 15:22
@felixweinberger felixweinberger added auth Issues and PRs related to Authentication / OAuth documentation Improvements or additions to documentation labels Sep 23, 2025
auth_code = callback_server.wait_for_callback(timeout=300)
return auth_code, callback_server.get_state()
finally:
callback_server.stop()
Copy link
Member

Choose a reason for hiding this comment

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

we also want to stop it here though, so we're not holding on to the port. Agree it's better to have it in the finally as well, but I think we want it both places since we don't need the callback server after the callback is complete.

@pcarleton pcarleton added the needs more work Not ready to be merged yet, needs additional changes. label Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth Issues and PRs related to Authentication / OAuth documentation Improvements or additions to documentation needs more work Not ready to be merged yet, needs additional changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants