Skip to content

`config.GetHTTPClient()` is not threadsafe

Consider this method:

func (c *Config) GetHttpClient() *client.HttpClient {
	if c.HttpClient != nil {
		return c.HttpClient
	}

	client := client.NewHTTPClient(...)

	c.HttpClient = client

	return client
}

It's a very standard check-then-modify pattern that is not thread-safe at all. In gitlab-shell, it's fine. In gitlab-sshd, it's not, since we share the same Config struct across multiple goroutines, similar to what's happening in !454 (comment 547791779)

We can fix this particular instance quite easily, but we should also audit the codebase for similar thread-safety issues.