-
Notifications
You must be signed in to change notification settings - Fork 244
Description
At the moment, if I call Conn.RemoveSignal(ch) to stop receiving signals, the channel remains open and it is my responsibility to close it. If the connection is closed though, then godbus closes the channel (via defaultSignalHandler.Terminate()).
This is a problem for code structured like the following (error handling elided):
func handleSignals(ctx *context.Context, conn *dbus.Conn) {
ch := make(chan *dbus.Signal)
defer close(ch)
conn.Signal(ch)
defer conn.RemoveSignal(ch)
for {
select {
case <-ctx.Done():
return
case sig := <- ch:
// do something with the signal
}
}
}
If the context is done before the connection closes, then we'll clean up the channel correctly without leaking. If the connection closes first however, then we'll get a panic because the channel was closed twice.
Given that the connection can be closed outside the control of the application (e.g. if dbus-daemon drops the socket connection, then godbus will call Close() itself), it's not clear that it is possible to write correct code under these semantics.
It's not clear that this can be fixed without breaking compatibility though: if RemoveSignal() starts closing the channel, that will likely cause panics in code that expects the channel to remain open. And if Close() stops closing the channel, then it will likely lead to zombie goroutines in applications that were relying on godbus to close the channel during cleanup.