Mutex debugging question

353 views
Skip to first unread message

Lars Pensjö

unread,
Sep 29, 2011, 9:23:34 AM9/29/11
to golan...@googlegroups.com
I am looking for "idiomatic" suggestions on this. Suppose there are many goroutines, processes and mutexes. I know want to ensure that the use of the locks are not bad. With "bad", I mean for example avoiding blocking, and that a lock shall never be requested for a specific mutex if it has already been given to the goroutine. I am not an expert on semaphore techniques, and the question (as explained below) is in some parts specific to Go.

What I have done is to create my own replacement of the "sync" package, with the same interface as "sync". It is an add-on to "sync", adding debug features. That way, I can simply import my own package instead of sync, and use the name "sync" in the important statement. I now have the possibility to produce statistics as well as some more advanced information (what function it was that released a lock I had to wait for).

I would like to be able to detect the case when a goroutine calls a lock twice (and thus blocks on itself), but I am uncertain on how to design such a test. Preferably, it should not change the interface from "sync", so that I easily can put the original "sync" back again when validation is finished. It may seem trivial to detect when a goroutine blocks itself, But not if you have a lot of them and do not keep perfect track of which ones are blocked from an expected behaviour and which ones of them are blocked from design errors.

For acquisition of more than one lock, I have a pre defined order that is supposed to be followed. I would like to be able to detect when this order is not adhered to by a goroutine.

I always use the same goroutine for lock and unlock, which should make it possible for some automatic testing.

Dmitry Vyukov

unread,
Sep 29, 2011, 11:38:07 AM9/29/11
to golan...@googlegroups.com
On Thu, Sep 29, 2011 at 5:23 PM, Lars Pensjö <lars....@gmail.com> wrote:
I am looking for "idiomatic" suggestions on this. Suppose there are many goroutines, processes and mutexes. I know want to ensure that the use of the locks are not bad. With "bad", I mean for example avoiding blocking, and that a lock shall never be requested for a specific mutex if it has already been given to the goroutine. I am not an expert on semaphore techniques, and the question (as explained below) is in some parts specific to Go.

What I have done is to create my own replacement of the "sync" package, with the same interface as "sync". It is an add-on to "sync", adding debug features. That way, I can simply import my own package instead of sync, and use the name "sync" in the important statement. I now have the possibility to produce statistics as well as some more advanced information (what function it was that released a lock I had to wait for).

I would like to be able to detect the case when a goroutine calls a lock twice (and thus blocks on itself), but I am uncertain on how to design such a test. Preferably, it should not change the interface from "sync", so that I easily can put the original "sync" back again when validation is finished. It may seem trivial to detect when a goroutine blocks itself, But not if you have a lot of them and do not keep perfect track of which ones are blocked from an expected behaviour and which ones of them are blocked from design errors.


In order to do that you need some sort of goroutine identity, but I don't see how you can obtain it (even indirectly).
Manually assigning unique ids to all goroutines and then passing it everywhere does not look like an option.

One solution is to patch the runtime, what you need is just:

func runtime.Gid() uint64

void runtime.Gid(uint64 id)
{
  id = g->goid;
  FLUSH(&id);
}

Then, you will be able to store in the mutex id of owner goroutine.



For acquisition of more than one lock, I have a pre defined order that is supposed to be followed. I would like to be able to detect when this order is not adhered to by a goroutine.

Here you also need some sort of goroutine local data (to hold a set of currently acquired mutexes). There is no way to do that.
However, if you apply the above patch, then you can use a global map of goroutine-id to set of acquired mutexes, something along the lines of:

var mu sync.Mutex
var locks map[uint64]map[uint64]bool

func (m *Mutex) Lock() {
  mu.Lock()
  myLocks := locks[runtime.Gid()]
  mu.Unlock()
  // check that myLocks does not contain incorrect locks
  // add m to myLocks
  ...

tux21b

unread,
Sep 29, 2011, 12:06:26 PM9/29/11
to golan...@googlegroups.com
I do not know your task, but I think you are doing it wrong... :D

Whenever you access a mutex (even for reading) you need to update it in an atomic way, which is quite slow and not scale-able. Also, as you have already found out, Mutexes are vulnerable to starvation and things like that. Which doesn't mean that they are generally a bad thing. For example, if you have a predefined interface which handles every access to a rarely used resource, a Mutex might be an easy solution to archive your goal.

But the general advice to write concurrent programs, is to avoid concurrency as much as possible. You should always try to keep variables with shared mutability to a bare minimum. It`s much faster, easier and less error-prone if you have only one goroutine which is responsible for your data. You can still merge those isolated variables at the end or in specific intervals, if you need to. That's also one of the reason why go maps are not goroutine-safe by default. Go channels are an easy way (but not necessarily the fastest) to avoid the need for sharing. "Do not communicate by sharing memory; instead, share memory by communicating." (Effective Go).

We all have once learned how to encapsulate data in abstract objects in those OO languages, but I think with Go, it's now time to rethink that design a bit and try to keep that data which belongs together in a single goroutine, while keeping everything else out.

-christoph

unread,
Sep 29, 2011, 12:28:23 PM9/29/11
to golang-nuts
On Sep 29, 3:23 pm, Lars Pensjö <lars.pen...@gmail.com> wrote:
> I would like to be able to detect the case when a goroutine calls a lock
> twice (and thus blocks on itself), but I am uncertain on how to design such
> a test.

Just a note: Double-locking within the same goroutine is a valid
operation as long there is another goroutine which will call Unlock().

unread,
Sep 29, 2011, 12:59:53 PM9/29/11
to golang-nuts
On Sep 29, 6:06 pm, tux21b <tux...@gmail.com> wrote:
> But the general advice to write concurrent programs, is to avoid concurrency
> as much as possible.

This appears to be a self-contradictory sentence.

> We all have once learned how to encapsulate data in abstract objects in
> those OO languages, but I think with Go,

In my opinion, Go isn't against data encapsulation. Go seems to
disagree about things like OO inheritance, superfluous pre-
determination of structure, or excessive code complexity.

> it's now time to rethink that
> design a bit and try to keep that data which belongs together in a single
> goroutine, while keeping everything else out.

In my opinion, it depends on the problem being solved and other
constraints.

Paul Borman

unread,
Sep 29, 2011, 1:04:45 PM9/29/11
to ⚛, golang-nuts
On Thu, Sep 29, 2011 at 9:59 AM, ⚛ <0xe2.0x...@gmail.com> wrote:
On Sep 29, 6:06 pm, tux21b <tux...@gmail.com> wrote:
> But the general advice to write concurrent programs, is to avoid concurrency
> as much as possible.

This appears to be a self-contradictory sentence.

Taken out of context like this you are right.  In context, however, Christoph's following sentence expands and explains he means concurrent access to a shared resource (such as memory).  He is absolutely spot on with this statement.

    -Paul 

Lars Pensjö

unread,
Sep 29, 2011, 1:58:35 PM9/29/11
to golan...@googlegroups.com
Thanks for some good advice tux21b.

The proposal to merge variables "afterwards" is something I shall consider, it is interesting. However, I may need to rethink some of the design for this. There is a risk that it adds overhead, and my implementation is very sensitive to performance. I suppose using channels is one way to implement this?

Using channels as a way to sharing memory is a very nice idiom, but it does work less well when you have many readers. In my case, I have thousands of asynchronous readers that indirectly depends on some of the protected data.

Lars Pensjö

unread,
Sep 29, 2011, 2:06:49 PM9/29/11
to golan...@googlegroups.com
On Thursday, 29 September 2011 17:38:07 UTC+2, Dmitry Vyukov wrote:

In order to do that you need some sort of goroutine identity, but I don't see how you can obtain it (even indirectly).
Manually assigning unique ids to all goroutines and then passing it everywhere does not look like an option.

Spot on. It would just add too much of overhead for something I only want temporarily.
 
One solution is to patch the runtime, what you need is just:

I was afraid of that. I hesitate to do my own changes to the runtime. The way I understand it, there are no plans to extend the standard runtime support with goroutine specific data. I suppose you should not need it in general. But for debugging and validation purposes, it can come handy.

Dmitry Vyukov

unread,
Sep 29, 2011, 2:07:05 PM9/29/11
to golan...@googlegroups.com
You may consider using COW (copy-on-write) data structure for that:

var data *Data
var mu sync.Mutex

// writers
mu.Lock()
newdata := makeNewData(data)
atomic.StorePointer(&data, newdata)
// old data will be picked up by GC when old readers are done with it
mu.Unlock()

// readers
d := atomic.LoadPointer(&data)
process(d)

It works extremely well in contexts with lots of readers.

Dmitry Vyukov

unread,
Sep 29, 2011, 2:15:26 PM9/29/11
to golan...@googlegroups.com
The debugging functionality could be provided by the sync package itself (when you set a special env var or something). But the problem is that sync.Mutex is not a mutex, it's a semaphore. So unfortunately it's impossible.

Per goroutine data is evidently a useful thing. It is proven by the language runtime which extensively uses it instead of requiring a user to pass contexts manually everywhere.

Lars Pensjö

unread,
Sep 30, 2011, 6:31:05 AM9/30/11
to golan...@googlegroups.com
On Thursday, 29 September 2011 20:07:05 UTC+2, Dmitry Vyukov wrote:
atomic.StorePointer(&data, newdata)
...
d := atomic.LoadPointer(&data)


Embarrasing question, but I can't find atomic.StorePointer() and atomic.LoadPointer() in the documentation?

Also, import "unsafe" fails on the golang.org home page test dialog. Maybe that is on purpose.

tux21b

unread,
Sep 30, 2011, 6:41:39 AM9/30/11
to golan...@googlegroups.com
On Friday, September 30, 2011 12:31:05 PM UTC+2, Lars Pensjö wrote:
Embarrasing question, but I can't find atomic.StorePointer() and atomic.LoadPointer() in the documentation?

It was added after the latest release (r60). So you will need at least the "weekly" version of Go:

 

Also, import "unsafe" fails on the golang.org home page test dialog. Maybe that is on purpose.
 
Probably :D 

Kyle Lemons

unread,
Sep 30, 2011, 11:49:11 AM9/30/11
to golan...@googlegroups.com
Probably :D 
Definitely ;-).  It's also omitted from app engine, among other things (os.Open, exec.Command, etc).
Reply all
Reply to author
Forward
0 new messages