Remove redundant event loop shutdown check in Netty4TcpChannel

[original-brownbear]

This check isn't necessary any longer. We safely close all channels before shutting down the even loop groups so we can not run into the situation that a listener passed to writeAndFlush isn't completed. We also don't have this check on the http channel sending. -> remove the check that can cause a bug by double invoking the listener when the event loop concurrently closes.

closes #95759

[elasticsearchmachine]

Pinging @elastic/es-distributed (Team:Distributed)

[DaveCTurner]

Yep seems better to shut these things down gracefully indeed. Can we assert channel.eventLoop().isShutdown() == false to make sure our shutdown process actually works as described?

[original-brownbear]

Can we assert channel.eventLoop().isShutdown() == false to make sure our shutdown process actually works as described?

We can't, the fact that we can run into this situation is exactly what's causing the linked test failure.
We can have a non-transport thread send a message and either run into a closed channel (which will fail the listener right away) or less have the event loop shut down concurrently so that it was still open when calling writeAndFlush but then got closed before we do the check.
We don't have the guarantee that the transport threads are stopped last is what it comes down to.

[DaveCTurner]

Ah ok sorry I missed that this could be running outside the event loop. Does assert (channel.eventLoop().inEventLoop() && channel.eventLoop().isShutdown()) == false; hold? Eh forget that, how could we be in the event loop if it's shut down?

[original-brownbear]

I assume you mean:

assert (channel.eventLoop().inEventLoop() == false || channel.eventLoop().isShutdown()) == false;

? I As far as I can tell there's no guarantee that that's true. Only if we checked for isTerminated would it be true but it also seems redundant to assert that?

how could we be in the event loop if it's shut down?

I guess shut-down just means no new tasks are accepted. Terminated means all tasks are done.

[Tim-Brooks]

LGTM

[original-brownbear]

Thanks Tim + David! Merging this since you're on PTO David :)