varnishcache / varnish-cache

Varnish Cache source code repository
https://www.varnish-cache.org
Other
3.56k stars 366 forks source link

varnishd: always shutdown both ends to a backend #4064

Closed asadsa92 closed 4 months ago

dridi commented 4 months ago

Looks like a good catch to me, but should we not handle the pfd as in vcp_handle() here?

The loop below is waiting for waiters to notice the shutdown and call vcp_handle().

dridi commented 4 months ago

Actually on this topic, this sleeping loop is something we should move as a CLI hook, waiting asynchronously for connection pools to be freed before deleting them for good.

nigoroll commented 4 months ago

The loop below is waiting for waiters to notice the shutdown and call vcp_handle().

Ah, this makes sense, thank you.

FWIW, @asadsa92 please add such explanations to the issue description or commit message. Leaving it to reviewers to figure out for themselves is no good use of their time.

this sleeping loop is something we should move as a CLI hook, waiting asynchronously for connection pools to be freed

yes.

dridi commented 4 months ago

this sleeping loop is something we should move as a CLI hook, waiting asynchronously for connection pools to be freed

yes.

I will follow up on that, I know someone who can work on this.

asadsa92 commented 4 months ago

FWIW, @asadsa92 please add such explanations to the issue description or commit message. Leaving it to reviewers to figure out for themselves is no good use of their time.

Sorry about that. This is kind of the gist of the issue, once the read side of the socket is shutdown the waiter issues a notification which calls the vcp_handle to clean up. Not closing the read side result in no notification from the waiter.

dridi commented 4 months ago

bugwash: merge