1
\$\begingroup\$

In golang I am finding that there is a definite pressure in the language itself and built-in libraries to do things in a canonical way. Please can someone critically look at these two ways I am considering using to provide transactional synchronisation of state manipulation within a library and provide me with some feedback.

All state manipulation functions of the library (i.e. all but a couple helper functions) would use this pattern and there would be over a dozen non-trivial functions that require transactional operation. Method 1 is what I would usually have done in C# but the official golang blog almost seems to be implying this is an anti-pattern.

Method 1 - C# lock pattern https://play.golang.org/p/XS14fM_XUg

gobyexample.com/mutexes

package main

import (
    "fmt"
    "sync"
)

type ILibraryState interface {
    DoSomething(arg int) error
}

type LibraryState struct {
    lock *sync.Mutex
    // Some private state
}

func (l *LibraryState) DoSomething(arg int) error {
    // Do some arg check

    // Transactional synchronisation
    l.lock.Lock()
    defer l.lock.Unlock()

    // Do something
    fmt.Println("Hello, playground")
    return nil;
}

func NewLibrary() ILibraryState {
    lib := LibraryState{}
    lib.lock = &sync.Mutex{}
    return &lib
}

func main() {
    l := NewLibrary()
    l.DoSomething(5)
}

Method 2 - Hidden goroutine and single command queue https://play.golang.org/p/3vWQJDswJm

package main

import (
    "fmt"
)

type ILibraryState interface {
    DoSomething(arg int) error
}

type LibraryState struct {
    command  chan (interface{})
    response chan (interface{})
    // Some private state
}

type commandDoSomething struct {
    arg int
}

func (l *LibraryState) DoSomething(arg int) error {
    // Do some arg check

    // Perform operation transactionally
    l.command <- commandDoSomething{arg}
    res := <-l.response
    if res == nil {
        return nil
    }
    return res.(error)
}

func (l *LibraryState) reallyDoSomething(arg int) {
    // Do something
    fmt.Println("Hello, playground")
    var err error = nil
    l.response <- err
}

func (l *LibraryState) processCommands() {
    for {
        switch c := (<-l.command).(type) {

        case commandDoSomething:
            l.reallyDoSomething(c.arg)
        }
    }
}

func NewLibrary() ILibraryState {
    lib := LibraryState{}
    lib.command = make(chan (interface{}))
    lib.response = make(chan (interface{}))
    go lib.processCommands()
    return &lib
}

func main() {
    l := NewLibrary()
    l.DoSomething(5)
}
\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

Go does have a strong opinion on the use of mutexes vs. channels. In fact, the sync package states: "Other than the Once and WaitGroup types, most are intended for use by low-level library routines. Higher-level synchronization is better done via channels and communication." (See https://golang.org/pkg/sync/ )

This opinion clearly implies your first implementation is suboptimal. I agree, so I'm not going to even review that implementation.

The second implementation is more intriguing, it uses channels, but it's got issues.... and it's not a "canonical" example of what I would expect with a "communication" implementation for concurrency in Go.

The "Stateful" go-routine is one which only that single go-routine accesses, or modifies the state. All interaction with the state is done by communicating with that routine, and communication happens through channels.

You have one channel to communicate the request for accessing the state, and another for returning the response. This is a problem... The request channel command is OK, but the response channel is broken. If you have multiple concurrent commands issued, you won't be able to send the right response to the right command, and things will get mixed up.

The solution to this is to dedicate a channel for each command to get the responses on. Add the channel to the commandDoSomething struct, so that your code looks something like:

func (l *LibraryState) DoSomething(arg int) error {

    // dedicated response channel for communication (of error type, not interface{})
    resp := make(chan error, 1)

    // Perform operation transactionally
    l.command <- commandDoSomething{arg, resp}
    res := <-response
    return res
}

Now your state handler would look like:

func (l *LibraryState) reallyDoSomething(arg int, respondto chan<- error) {

    defer close(respondto)

    // Do something
    fmt.Println("Hello, playground")
    var err error = nil
    respondto <- err
}

and be called like:

for {
    switch c := (<-l.command).(type) {

    case commandDoSomething:
        l.reallyDoSomething(c.arg, c.respondto)
    }
}

Note that you can remove the l.response channel completely.

Edit: Note that by dedicating a response channel for each command, you can do things like return the actual channel from the DoSomething function, like:

func (l *LibraryState) DoSomething(arg int) <-chan error {

    // dedicated response channel for communication (of error type, not interface{})
    resp := make(chan error, 1)

    // Perform operation transactionally
    l.command <- commandDoSomething{arg, resp}
    return resp
}

and as a consequence, "clients" can call the DoSomething method and then potentially do other work while the state is changing, and get the possible error condition later... like:

resp := DoSomething(input)
//... work on other activities
if err := <-resp; err != nil {
    // handle error - perhaps undo some previous optimistic code....
}
\$\endgroup\$
5
  • \$\begingroup\$ Are you sure about the indeterminancy of the response channel? I thought that if the channels are zero length a goroutine can't post onto command until another is blocking on receiving from it. If I am correct about this, doesn't this mean that no goroutine other than the first requester can move past offering a command until the first one has collected it's response? \$\endgroup\$ Feb 25, 2017 at 15:07
  • \$\begingroup\$ Hmm, you're right about the unbuffered channel, this does in fact make your code "right", but... (I blame the language itself for this) ... it's really hard to see when channels are buffered, or not, when you are in other parts of the code. I simply missed that, and it's a common mistake to make. You should comment your code clearly to indicate that there's no concurrency at all on the client side. I would still prefer my option because you can then return the individual channel from the DoSomething and let the client work asynchronously, let me edit.... \$\endgroup\$
    – rolfl
    Feb 25, 2017 at 15:11
  • \$\begingroup\$ Note that in your first stateful go-routines link it also uses a distinct channel for each command's response. \$\endgroup\$
    – rolfl
    Feb 25, 2017 at 15:20
  • \$\begingroup\$ Would it also be a good idea to use a channel of something like ICommand with an execute method so I don't need the switch statement or multiple command channels? Or do multiple command channels / multi select statement have an advantage? \$\endgroup\$ Feb 25, 2017 at 15:24
  • \$\begingroup\$ The multi-select option is a good one because then you don't need to cast the input commands at all. Again, there is a good example of it in the first link: gobyexample.com/stateful-goroutines - there are two commands implemented there, and they each have their own channel. The routine uses a select to identify channels with activity. \$\endgroup\$
    – rolfl
    Feb 25, 2017 at 15:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.