Skip to content

Make Server thread-safe with per-thread sessions - #1871

Open
dxdc wants to merge 1 commit into
tableau:developmentfrom
dxdc:development
Open

Make Server thread-safe with per-thread sessions#1871
dxdc wants to merge 1 commit into
tableau:developmentfrom
dxdc:development

Conversation

@dxdc

@dxdc dxdc commented Aug 24, 2026

Copy link
Copy Markdown

Motivation

Audit result from #1148: all HTTP traffic flows through a single requests.Session stored on Server, and requests.Session is not guaranteed to be thread-safe (psf/requests#2766 - connection pool and cookie state can be corrupted under concurrent use). Auth state (_auth_token, _site_id, _user_id, _site_url) is also written field-by-field with no lock, so a concurrent reader could observe a token paired with the wrong site during switch_site or re-sign-in.

The practical consequence was that the common pattern of sharing one Server across a ThreadPoolExecutor (e.g. bulk workbook downloads) was unsafe, and the workaround was one fully signed-in Server per thread.

Behavior change

  • Server.session now lazily returns a per-thread requests.Session created from session_factory, cached in threading.local. This is exactly the "one session per thread" guidance from the requests maintainers, applied transparently. Thread pool workers reuse their session (and its connection pool) across tasks.
  • _set_auth / _clear_auth are guarded by a lock so the (site, user, token) triple is always updated atomically. Reads stay lock-free.
  • Sign-out previously dropped cookie state by replacing the single shared session. That behavior is preserved for all threads via an epoch counter: _clear_auth bumps the epoch, and each thread lazily replaces its cached session on next use. In-flight requests on other threads are not disrupted, matching the old re-assignment semantics.
  • New Server.close() and context-manager support (with TSC.Server(...) as server:). Sessions created for any thread are tracked in a WeakSet (weak, so sessions of exited threads can still be garbage collected) and close() closes them all, releasing pooled connections. close() is transport-level only - it does not sign out - so it composes with the existing auth.sign_in() context manager, which already handles sign-out. The Server remains usable after close(); the epoch bump means any later call creates fresh sessions instead of hitting closed pools.
  • The thread-safety contract is documented in the Server docstring: share one instance freely, sign in (and call use_server_version()) before spawning workers, and note that session_factory may now be called once per thread.

No public API changes. All endpoint code already routed through the session property, so the change is confined to server.py. The one private-surface change is that Server._session no longer exists as an attribute.

New test/test_thread_safety.py:

  • test_each_thread_gets_its_own_session - N threads held live behind a barrier each see a distinct, per-thread-stable session object
  • test_session_factory_called_once_per_thread - factory runs exactly once per thread despite repeated session access
  • test_sign_out_invalidates_sessions_of_all_threads - after auth.sign_out(), both the signing-out thread and a still-alive worker thread get fresh sessions
  • test_concurrent_api_calls_use_per_thread_sessions - 8-worker ThreadPoolExecutor making 40 mocked users.get() calls; every request completes and carries the shared auth token
  • test_auth_state_is_set_atomically - reader threads hammer auth_token/site_id while the main thread cycles _set_auth/_clear_auth; no reader ever observes a mismatched token/site pair
  • test_close_closes_sessions_of_all_threads - close() closes the sessions of the constructing thread and all workers, and the server remains usable afterwards
  • test_close_does_not_sign_out - auth token survives close()
  • test_context_manager_closes_on_exit - __enter__ returns the server, __exit__ closes sessions

Verified the first two tests fail against the current development branch.

@salesforce-cla

Copy link
Copy Markdown

Thanks for the contribution! Before we can merge this, we need @dxdc to sign the Salesforce Inc. Contributor License Agreement.

@dxdc

dxdc commented Aug 24, 2026

Copy link
Copy Markdown
Author

signed the cla

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant