Skip to content

refactor: centralize thread-local cleanup into ThreadLocalCleaner - #1260

Open
bengbengbalabalabeng wants to merge 2 commits into
apache:mainfrom
bengbengbalabalabeng:refactor-unified-threadlocal-cleanup
Open

bengbengbalabalabeng wants to merge 2 commits into
apache:mainfrom
bengbengbalabalabeng:refactor-unified-threadlocal-cleanup

Conversation

@bengbengbalabalabeng

@bengbengbalabalabeng bengbengbalabalabeng commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Purpose of the pull request

What's changed?

  • Add a utility to clear all thread locals

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

@skytin1004 skytin1004 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I pulled the latest head and reproduced a concurrency failure in the new registry. While one thread was iterating cleanTasks in clearAllThreadLocals(), I registered another cleaner from a second thread. The finishing thread threw ConcurrentModificationException.

Because finish() now calls this registry directly, a successful read or write can fail during cleanup when another thread lazily initializes a utility class and registers its cleaner. Please make registration and iteration thread-safe, for example with CopyOnWriteArrayList or an immutable snapshot under synchronization, and add a deterministic concurrency regression test.

@bengbengbalabalabeng

Copy link
Copy Markdown
Contributor Author

I pulled the latest head and reproduced a concurrency failure in the new registry. While one thread was iterating cleanTasks in clearAllThreadLocals(), I registered another cleaner from a second thread. The finishing thread threw ConcurrentModificationException.

Because finish() now calls this registry directly, a successful read or write can fail during cleanup when another thread lazily initializes a utility class and registers its cleaner. Please make registration and iteration thread-safe, for example with CopyOnWriteArrayList or an immutable snapshot under synchronization, and add a deterministic concurrency regression test.

Thanks for the review! I've switched the list to CopyOnWriteArrayList.

This branch has not been deployed

No deployments
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.

2 participants