Egon Elbre

Asserting Locks

One of the problems I’ve encountered few times is avoiding mistakes with threaded programs and locks.

Whether you should be using locks is a different discussion. There are plenty of places where other approaches, such as channels, can be much easier to read and understand.

A basic example of my problem can be seen below:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
package example

import (
	"io"
	"sync"
)

type Channel struct {
	mu      sync.Mutex
	clients map[string]io.ReadWriter
}

func (channel *Channel) Connect(name string, client io.ReadWriter) {
	channel.mu.Lock()
	defer channel.mu.Unlock()

	channel.broadcast(name + " connected")
	channel.clients[name] = client
}

func (channel *Channel) Disconnect(name string) {
	channel.mu.Lock()
	defer channel.mu.Unlock()
	channel.disconnect(name)
}

func (channel *Channel) disconnect(name string) {
	// channel.mu must be held
	delete(channel.clients, name)
	channel.broadcast(name + " disconnected")
}

func (channel *Channel) broadcast(message string) {
	// channel.mu must be held
	for name, client := range channel.clients {
		n, err := client.Write([]byte(message))
		if err != nil || n != len(message) {
			channel.disconnect(name)
		}
	}
}

See those comments “_channel.mu must be held_”. The more complicated the logic is, the harder it will be to ensure that these comments are being followed. This gets extra complicated when you have multiple locks involved.

Also, in some languages you don’t have a race or static checker so an easy fix is to implement a method that fails when the Mutex is not properly held. The fixed code would look like this:

So how do we implement it? Easy – track who owns the lock:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
package example

import (
	"io"
	"github.com/egonelbre/exp/sync2"
)

type Channel struct {
	mu      sync2.Mutex
	clients map[string]io.ReadWriter
}

func (channel *Channel) Connect(name string, client io.ReadWriter) {
	channel.mu.Lock()
	defer channel.mu.Unlock()

	channel.broadcast(name + " connected")
	channel.clients[name] = client
}

func (channel *Channel) Disconnect(name string) {
	channel.mu.Lock()
	defer channel.mu.Unlock()
	channel.disconnect(name)
}

func (channel *Channel) disconnect(name string) {
	channel.mu.MustOwn()
  
	delete(channel.clients, name)
	channel.broadcast(name + " disconnected")
}

func (channel *Channel) broadcast(message string) {
	channel.mu.MustOwn()
  
	for name, client := range channel.clients {
		n, err := client.Write([]byte(message))
		if err != nil || n != len(message) {
			channel.disconnect(name)
		}
	}
}

The code isn’t completely correct (a brief period where “MustOwn” panics with the wrong message), but it’s still helpful and can find bugs. The whole implementation only needs a way to identify the current thread/goroutine. In Go it is a little problematic, but doable.

There’s also the question of, what do we do when we want to Lock and then give the mutex to another thread without Unlocking. The problem can be solved by implementing a “Take” method that changes the owner.

The implementation is not limited to Go and can easily be re-implemented in C, D or Delphi – whatever languages you need to use.

That’s it. Hopefully it helps someone.