Skip to content

Conversation

ejona86
Copy link
Member

@ejona86 ejona86 commented Sep 30, 2025

The only caller of drain() just reset the interrupt flag. So, we can avoid the whole exception dance.

Backport of #12358

CC @benjaminp, FYI

…in (grpc#12358)

The only caller of `drain()` just reset the interrupt flag. So, we can
avoid the whole exception dance.
@ejona86 ejona86 requested a review from AgraVator September 30, 2025 20:30
@benjaminp
Copy link
Contributor

Why the backport? I don't think this fixes any bugs.

@ejona86
Copy link
Member Author

ejona86 commented Sep 30, 2025

There's actually a behavior change, of whether the executor runs or not, not just avoiding creating the exception. And it seemed getting any such changes out sooner is probably better for all new stub users.

We can let it sit for the next release though. I guess you just saw the inefficiency and decided to improve that, which really doesn't need to be backported.

@benjaminp
Copy link
Contributor

As far as I can tell, ThreadSafeThreadlessExecutor.drain() is only reachable from test code.

I made this change as prefactoring for the change I'm really interested in: #12372

@ejona86
Copy link
Member Author

ejona86 commented Sep 30, 2025

Oh, I see. I thought some of those methods were used elsewhere. Yeah, there's no point to this backport. (And I agree #12372 is important).

@ejona86 ejona86 closed this Sep 30, 2025
@ejona86 ejona86 deleted the backport-drain-InterruptedException-1.76 branch September 30, 2025 21:26
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