Thread safety

[GoogleCodeExporter]

What steps will reproduce the problem?

Read 
http://groups.google.com/group/golang-nuts/browse_thread/thread/7c5689e1e2fe531d

Original issue reported on code.google.com by 0xE2.0x9A.0x9B@gmail.com on 20 Oct 2010 at 12:18

[GoogleCodeExporter]

Hi,

I started the thread mentioned.  I've just been playing with the code to find 
the problem.  I'll attach my program that exposes the problem, but the issue is 
that for some reason there are two lines in internAtomRequest that, together, 
need to sit inside a Mutex to allow that function to be re-entrant. I whacked a 
mutex into the Conn struct and then used it in that function.  I tried moving 
the lock to lower level calls in xdg.go but it won't work unless I put the lock 
unlock around the two calls as shown below.   My revised version of that 
function looks like this:

 func (c *Conn) InternAtomRequest(OnlyIfExists bool, Name string) Cookie {
    b := c.scratch[0:8]
    n := 8
    n += pad(len(Name) * 1)
    put16(b[2:], uint16(n/4))
    b[0] = 16
    if OnlyIfExists {
        b[1] = 1
    } else {
        b[1] = 0
    }
    put16(b[4:], uint16(len(Name)))
    c.mutex.Lock)(
    cookie := c.sendRequest(b)
    c.sendString(Name)
    c.mutex.Unlock()
    return cookie
}

Original comment by tea...@gmail.com on 20 Oct 2010 at 2:48

Attachments:

• foo.go

[GoogleCodeExporter]

Oh.. and if you take the other mutex out of foo.go, you'll also need to change 
InternAtomReply to look like this:

func (c *Conn) InternAtomReply(cookie Cookie) (*InternAtomReply, os.Error) {
    c.reply_mutex.Lock()
    b, error := c.waitForReply(cookie)
    c.reply_mutex.Unlock()
    if error != nil {
        return nil, error
    }
    v := new(InternAtomReply)
    v.Atom = Id(get32(b[8:]))
    return v, nil
}

Original comment by tea...@gmail.com on 20 Oct 2010 at 2:59

[GoogleCodeExporter]

I hope you won't take the following too negatively, but I have to mention it: 
You have a long way to go before you are able to write correct threaded code. 
Once you are trained, you will almost immediately notice that if 
"InternAtomRequest" is potentially called from multiple goroutines in parallel 
then "c.scratch" is a shared mutable variable. So, you need to execute 
"c.mutex.Lock()" a bit sooner.

Original comment by 0xE2.0x9A.0x9B@gmail.com on 20 Oct 2010 at 3:03

[GoogleCodeExporter]

No offence taken at all.  I actually though c.scratch would be the source of 
the error originally, but I couldn't see where it was being written to, only 
slices taken from it.  The versions of the code I've posted are only what I 
found by trial and error with my specific program - I haven't really had time 
today to look into the inner workings of x-go-binding. 

Original comment by tea...@gmail.com on 20 Oct 2010 at 5:49