anz-bank / pkg

Common ANZ Go packages
https://pkg.go.dev/github.com/anz-bank/pkg
Apache License 2.0
1 stars 9 forks source link

race condition in the pkg/health package #79

Closed JamesBLewis closed 1 year ago

JamesBLewis commented 1 year ago

I found a segmentation fault in the health package but I don't have write access to it as I think I need to be added to the anz-bank GitHub org.

The trigger for this segmentation fault is when a race condition between the calls to health.RegisterWithGRPC and health.RegisterWithHTTP results in the second go routine to reach line github.com/anz-bank/pkg/health/default.go#45 to reference a nil pointer if the first go routine has not finished creating the DefaultServer.

An example when this guaranteed to occur is if the first go routine encounters an error creating the default server. In my case I had provided an invalid value for the health.RepoURL field. This problem is compounded by the fact that the seg fault can kill the program before the offending error (the invalid RepoURL field) is logged. The result is a segmentation fault with often no clear cause.

What's the solution?

The simplest (and in my opinion best) solution is to add the following check to the RegisterWithGRPC and RegisterWithHTTP functions:

if DefaultServer == nil {
    return fmt.Errorf("Default server failed to start")
}

I have written a fix but do not have write access to this repo so cannot PR it.