Skip to content

Conversation

@WhyNotHugo
Copy link
Contributor

@WhyNotHugo WhyNotHugo commented Oct 16, 2025

Implement a Client.Closed() to be used by clients to monitor when a connection is closed.

@WhyNotHugo
Copy link
Contributor Author

I've implemented by particular case in some other way, but this sounds useful to have. I have no strong objections if you'd prefer to close this PR.

@emersion
Copy link
Owner

emersion commented Nov 1, 2025

I was wondering about the use-case for this. I initially thought I needed something similar for alps, but then figured it wouldn't help much:

  • The connection might be broken even when the reader goroutine is still alive, when a connection isn't explicitly closed by the remote and times out.
  • Even if the reader goroutine could perfectly detect when a connection is broken, checking for the closed channel introduced in this PR before sending a command would be racy.
  • A disconnected client only consumes a small amount of memory, its underlying FD is closed.

@WhyNotHugo
Copy link
Contributor Author

WhyNotHugo commented Nov 5, 2025

checking for the closed channel introduced in this PR before sending a command would be racy.

The channel is not intended to be checked before sending a message, but to re-connect automatically when the connection terminates (i.e.: in the case of NOTIFY).

@WhyNotHugo
Copy link
Contributor Author

WhyNotHugo commented Nov 5, 2025

Previously, the Notify.Overflow channel in #718 would also be closed when the connection dropped (a convenient side-effect). This allowed me to detect when a connection had been closed.

Having switched NOTIFY to use a UnilateralDataHandler for NOTIFYOVERFLOW events, I once again need this feature.

I don't think I have any other way of detecting when the connection has been closed. I'm only waiting for NOTIFY events, so never send any messages.

@emersion
Copy link
Owner

emersion commented Nov 9, 2025

Hm, right, that's a good point. I think NOTIFY is special here: with IDLE, clients could call IdleCommand.Wait to figure out when the connection was broken. Since NOTIFY isn't a long-running command, there's no equivalent.

}
}

func ExampleClient_Closed() {
Copy link
Owner

Choose a reason for hiding this comment

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

This example is racy (the main function doesn't typically wait for other goroutines to finish) and doesn't reflect real-world usage. Could we drop this example for now, and wait for NOTIFY to be implemented to add a Notify example which leverages Closed to figure out when the connection has been closed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Example redone.

Copy link
Owner

Choose a reason for hiding this comment

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

The example still feels a bit artificial - but let's revisit this with NOTIFY.

@WhyNotHugo WhyNotHugo force-pushed the closed branch 2 times, most recently from 4750b35 to 00fde73 Compare November 23, 2025 08:21
Implement a Client.Closed() to be used by clients to monitor when a
connection is closed.
Copy link
Owner

@emersion emersion left a comment

Choose a reason for hiding this comment

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

Thanks!

@emersion emersion merged commit cc7b09a into emersion:v2 Dec 4, 2025
1 check passed
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