Skip to content

Fix race condition in ConcurrentMultiSpanProcessor.shutdown() #4462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jomcgi
Copy link
Contributor

@jomcgi jomcgi commented Mar 6, 2025

Description

Fixed error during shutdown: 'cannot schedule new futures after shutdow'.
Modified shutdown sequence to handle executor already being in shutdown state.
Added direct processor shutdown when executor is unavailable.
Added regression test for executor shutdown race condition.

Fixes #4461

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • tox -e test-opentelemetry-sdk

Does This PR Require a Contrib Repo Change?

  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@jomcgi jomcgi force-pushed the fix-4461/concurrent-multi-span-processor branch 2 times, most recently from bfbab3a to 9d45683 Compare March 6, 2025 00:26
@jomcgi jomcgi changed the title [WIP] Fix race condition in ConcurrentMultiSpanProcessor.shutdown() Fix race condition in ConcurrentMultiSpanProcessor.shutdown() Mar 6, 2025
@jomcgi jomcgi marked this pull request as ready for review March 6, 2025 00:36
@jomcgi jomcgi requested a review from a team as a code owner March 6, 2025 00:36
Fixed error during shutdown: 'cannot schedule new futures after shutdow'.
Modified shutdown sequence to handle executor already being in shutdown state.
Added direct processor shutdown when executor is unavailable.
Added regression test for executor shutdown race condition.
@jomcgi jomcgi force-pushed the fix-4461/concurrent-multi-span-processor branch from 9d45683 to 3c31e1e Compare March 6, 2025 09:18
helderco added a commit to helderco/dagger that referenced this pull request Mar 19, 2025
helderco added a commit to dagger/dagger that referenced this pull request Mar 20, 2025
kpenfound pushed a commit to kpenfound/dagger that referenced this pull request Mar 21, 2025
@aabmass
Copy link
Member

aabmass commented Apr 4, 2025

Check my comment on #4461 I'm not sure this fixes the problem

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.

ConcurrentMultiSpanProcessor: cannot schedule new futures after shutdown
3 participants