Skip to content

Shutdown callback design refinements: TypeError double-invocation and shared timeout budget #9

Description

@scarnecchia

Context

Found in PR #6 cycle 3 review (standard + adversarial reviewers). Two related Nit findings about _run_shutdown_callback in all 5 telemetry modules.

1. TypeError-fallback can theoretically double-invoke callback

If a callback accepts timeout_millis but raises TypeError from within its body (not from argument binding), the except TypeError catches it and calls callback() again — a silent double-invocation.

Currently safe for OTel SDK callbacks (MeterProvider.shutdown and TracerProvider.shutdown raise TypeError at binding time with zero side effects), but fragile if custom callbacks are added.

Fix direction: Use inspect.signature to check parameter acceptance instead of try/except TypeError.

2. Shared timeout budget creates false-positive timeout window

The same SHUTDOWN_TIMEOUT_MILLIS (5000ms) is used both as the OTel callback's own timeout (callback(timeout_millis=5000)) and as the thread join timeout (thread.join(5.0)). If a callback uses its full 5s allocation, the thread may still be in epilogue when join(5.0) expires, logging a spurious "timed out" warning.

Fix direction: Use a slightly longer join timeout (e.g., SHUTDOWN_TIMEOUT_MILLIS / 1_000 + 0.5), or document as acceptable noise.

Affected packages

All 5: quote_cosharing, quote_overdispersion, url_overdispersion, signup_anomaly, account_entropy

Severity

Nit — theoretical edge cases, no current runtime impact.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions