proposal: sync: mechanism to select on condition variables

[bradfitz]

Sometimes condition variables are the best fit for a problem, and sometimes channels are. They don't compose well, though.

I've increasingly been finding myself in a position where I would like to select on both channel(s) and a condition variable. This has come up a few times in http2, where I use condition variables for flow control, but there are plenty of channels (including the net/http.Request.Cancel channel) in play too. Sometimes I need to both wait for flow control, or wait for a user to cancel their request, or the peer side to close their connection, etc.

I would love to be able to select on a condition variable, like if the sync.Cond type had a method:

// WaitChan returns a channel which receives a value when awoken
// by Broadcast or Signal. Unlike Wait, WaitChan does not unlock
// or lock the c.L mutex.
func (c *Cond) WaitChan() <-chan struct{} {
     // ....
}

The semantics would be the receiving from it acts like a runtime_Syncsemacquire on c.sema. We'd need to define what happens if another case in the select fires first. (does the semacquire get canceled?).

Thoughts?

[bradfitz]

/cc @ianlancetaylor @griesemer @randall77 @josharian

[ianlancetaylor]

It seems to me that you can turn a condition variable into a channel by writing

c := make(chan struct{})
go func() {
    cond.L.Lock()
    cond.Wait()
    c <- struct{}{}
}()

I'm sure I'm missing something, but what?

[bradfitz]

Yeah, except then I can leak goroutines if I'm not careful. I also pay the additional 4+ KB cost per select, which can be blocking for some time.

[josharian]

Stupid question, but why would it be ok for WaitChan not to lock or unlock c.L? Relatedly, for Ian's code sample, why isn't a loop checking condition required?

[bradfitz]

I would still reacquire the mutex afterwards.

Instead of this example from our docs:

c.L.Lock()
for !condition() {
    c.Wait()
}
... make use of condition ...
c.L.Unlock()

I would write:

c.L.Lock()
for !condition() {
    c.L.Unlock()
    select {
    case <-ctx.Done():
    case <-c.WaitChan():
    }
    c.L.Lock()
}
... make use of condition ...
c.L.Unlock()

[josharian]

My expertise in this area is very limited, but as I understand it, it is important that Wait atomically unlock c.L and suspends execution. Maybe in your sample code this will occur, in that Unlock is immediately followed by select, and we always have cooperative scheduling, but in the event that someone added other code just before the select in order to prepare for the select (e.g. calling a function to decide whether to nil out a channel), then we have a race. Seems like a footgun. And if we moved to pre-emptive scheduling (or used another toolchain), then there might be no way to write the code correctly.

I might be wrong about all of this, though.

[bradfitz]

There's nothing atomically special about the Cond.Wait method. It's just an unlock, wait, and lock:

https://github.com/golang/go/blob/release-branch.go1.7/src/sync/cond.go#L53

I don't see how this is any more of a footgun than anybody using the sync package in general. Or anybody using two goroutines and sharing global state without the sync package. Bad Go code is full of data races.

I'm proposing this as a mechanism for people who do know what they're doing. Not as some recommended high-level mechanism.

[ianlancetaylor]

@josharian I think what you are thinking of is that it is critical that a condition variable not miss a signal/broadcast in between the unlock and the goroutine suspension. Other than that there is no implied atomicity. A simple implementation of condition variables can easily make that mistake--unlock, check signal, (receive unnoticed signal), suspend. Our implementation doesn't have that problem. The required atomicity is in runtime.notifyListWait, via the internal lock it acquires.

[minux]

A more generic solution is perhaps for the compiler to pass stack frame size hints to the runtime. for example, in Ian's example, the compiler could recognize that the function will usually use very little stack, even less than the minimum stack, so it could hint the runtime to start the goroutine with a smaller stack. In general, if we make the compiler determine the initial stack size for a new goroutine, could that solve at least half of the problem? (goroutine leaking might still occur, but that's another issue.)

[bradfitz]

Kinda. Really I want a magic type of hchan (sentinel value of hchan.elemtype) which makes hchan.buf mean it's a pointer to a notifyList, and have the select implementation on entry to select do the notifyListAdd, and then if separate case fires in select, we do something like a notifyListRemove, and unregister that waiter.

Otherwise I'm afraid of the leaks where my condition variable is woken up (because, say, I selected on my context.Done()), but then I find that my condition is satisfied and I exit the function, and never wait on the condition variable again. I need to tell the cond.Wait running in the goroutine to stop. Which makes me think it's better done in the implementation of select with a special channel type.

This wouldn't be a language change. But it would involve modifying the runtime to support an API addition.

[minux]

could you design a user facing sync.Cond API for this?

we need to automatically lock cond.L when the channel
unblocks and the unlock needs to happen for the goroutine
that is actually winning the race. This seems too magical
to me. Maybe I'm missing something.

[bradfitz]

@minux, I covered all this above.