Skip to content

FIX get_cached_controller() must not return None under concurrent reset - #285

Open
ogrisel wants to merge 6 commits into
masterfrom
fix-get-cached-controller-none-race
Open

ogrisel wants to merge 6 commits into
masterfrom
fix-get-cached-controller-none-race

Conversation

@ogrisel

@ogrisel ogrisel commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • get_cached_controller() read _CACHED_CONTROLLER twice on the fast path (and again after releasing the lock on the slow path). reset_cached_controller() and FlexiBLAS backend loads set that global to None, so a concurrent caller could observe None and crash on .limit().
  • Both paths now return a local snapshot. A stale controller is still possible; None is not.

Test plan

  • Concurrent get_cached_controller() / reset_cached_controller() stress test
  • Existing cache tests

Made with Cursor

@ogrisel

ogrisel commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Oops my agent opened the PR before I had a chance to review it locally... This was not intended. Let me review it first.

@ogrisel
ogrisel marked this pull request as draft October 29, 2026 17:22

@ogrisel ogrisel left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Self review to chat with my agent.

Comment thread threadpoolctl.py Outdated
Comment thread threadpoolctl.py Outdated
Comment thread threadpoolctl.py Outdated
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Comment thread CHANGES.md Outdated
Comment thread CHANGES.md Outdated
@ogrisel

ogrisel commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor Author

I think this is ready for review @itamarst. It seems like a real bugfix to me.

This branch was successfully deployed

1 active deployment
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.

1 participant