Way to handle errors in goroutine. Is this safe?

[kr c]

package main

import (
        "errors"
        "log"
        "time"
)

func main() {
        err := f()
        if err != nil {
                log.Fatal(err)
        }
}

func f() (err error) {
        var rerr error

        defer func() {
                if rerr != nil {
                        err = rerr
                }
        }()

        go func() {
                rerr = longTask()
        }()

        time.Sleep(time.Second * 2)
        return errors.New("f error")
}

func longTask() error {
        time.Sleep(time.Second * 1)
        return errors.New("long task error")
}

https://play.golang.org/p/mqpT8XBbeiN

Hi, Everyone! I'm newbie in go. Help needed.

As far as I learned, gorutines should pass "value" through channels.

There's more safe way to handle this kind of stuffs with using waitgroup, context, channels....I thought, "sharing variable reference in gorutine" is dangerous because of problems like transacion issue.

But what if that value is just single error? Is it still unsafe or may cause panic?

"longTask" in examples is a task that I can garantee it will eventually end.

I know little about low level programming. So not sure if I can write this kind of code.

[Ian Lance Taylor]

On Sat, Dec 7, 2019 at 9:12 AM kr c <xnzm...@gmail.com> wrote:
>
> https://play.golang.org/p/mqpT8XBbeiN
>
> Hi, Everyone! I'm newbie in go. Help needed.
> As far as I learned, gorutines should pass "value" through channels.
> There's more safe way to handle this kind of stuffs with using waitgroup, context, channels....I thought, "sharing variable reference in gorutine" is dangerous because of problems like transacion issue.
>
> But what if that value is just single error? Is it still unsafe or may cause panic?
>
> "longTask" in examples is a task that I can garantee it will eventually end.
> I know little about low level programming. So not sure if I can write this kind of code.

That program is not safe. There is a race condition between writing
to rerr in the goroutine and reading rerr in the deferred function. I
would expect that the race detector would report a problem with this
program (https://golang.org/doc/articles/race_detector.html).

Use a channel.

Ian

[Jake Montgomery]

As Ian pointed out, the code you provided has a race condition. Using timers to try to avoid a race like you do here may work  most of the time, but it is not rigorously correct. When dealing with concurrency is is critical to always be 100% rigorusly "correct". Otherwise you will eventually encounter a situation where you manifest a race and potentially produce behavior that can be difficult to replicate and a night mare to diagnose. So, to summarize, "close enough" works for some things, but IMHO, never for concurrency.

There are many ways to handle your code correctly in go. Fortunately go provides some really reliable and safe ways to handle concurrency. The simplest would be a channel:

package main

import (
    "errors"
    "log"
    "time"
)

func main() {
    err := f()
    if err != nil {
        log.Fatal(err)
    }
}

func f() (err error) {

    errorChan := make(chan error)

    go func() {
        errorChan <- longTask()
    }()

    err = <-errorChan
    return err

}

func longTask() error {
    time.Sleep(time.Second * 1)
    return errors.New("long task error")
}

https://play.golang.org/p/2bJeaD7p93K

This should work correctly and is easy to understand and read. Of course, in this particular example , there is no real point to even using a goroutine.

In your original code, I found it really confusing the way you retunred one error, then potentially changed it based on the defer. Thats the kind of thing that makes code hard to read, and bugs easy to miss.