Skip to content

Remove assertions for DrainManagerImpl to be run on main thread.#31701

Closed
furlongmt wants to merge 1 commit intoenvoyproxy:mainfrom
furlongmt:master
Closed

Remove assertions for DrainManagerImpl to be run on main thread.#31701
furlongmt wants to merge 1 commit intoenvoyproxy:mainfrom
furlongmt:master

Conversation

@furlongmt
Copy link
Copy Markdown
Contributor

Remove assertions for DrainManagerImpl to be run on main thread. These callbacks can be run on the worker thread so we must only check that the dispatcher is thread safe

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

…e callbacks can be run on the worker thread so we must only check that the dispatcher is thread safe
Copy link
Copy Markdown
Contributor

@jmarantz jmarantz left a comment

Choose a reason for hiding this comment

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

this looks good.

WDYT about adding a unit test that fails without this change? It'd have to use real threads.

@jmarantz jmarantz self-assigned this Jan 8, 2024
@jmarantz
Copy link
Copy Markdown
Contributor

jmarantz commented Jan 8, 2024

superceded by #31711 so that @furlongmt doesn't need to read the book of knowledge about DCO

@jmarantz jmarantz closed this Jan 8, 2024
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