How to copy a struct which contains a mutex?

[appro...@codinglife.me]

In package sync's document:

Values containing the types defined in this package should not be copied.

[Staven]

On Thu, Apr 07, 2016 at 02:40:33AM -0700, appro...@codinglife.me wrote:
> In package sync's document:
> Values containing the types defined in this package should not be copied.

In the words of a great poet: so don't do that.

Possibly write a copying function that copies only fields that aren't
a mutex (you probably want to acquire the mutex first).

[Roberto Zanotto]

Do you have a piece of code that tries to accomplish something or is it a purely theoretical question?

You just don't copy a mutex value, because it doesn't really make sense.

[dja...@gmail.com]

do not use structs which contains a mutex, use structs with pointer to mutex

[Andrew Gerrand]

It's fine to put a sync.Mutex in a struct—it's a common thing to do—just be mindful not to copy the struct (pass a pointer instead).

[Dave Cheney]

Hello,

What is the problem you are trying to solve? Ie, why do you need to copy a structure that contains a mutex?

In my experience copying arbitrary structures is an anti pattern as you're only making a shallow copy of the contents. Maps, slices, channels, pointers and so on will still point to their original location. So, wanting to do a shallow copy is usually wrong, and wanting to make a deep copy is complex in the context of circular reference and types which are private.

Can you explain a bit more about the background of your question and maybe we can give a more useful answer.

Thanks

Dave

[Paul Borman]

Okay, there are real uses cases for this, I just hit one the other day.  I had a structure (call it a) with a mutex.  I needed to clone it.  I came up with three ways:

1) Copy it with the mutex locked and then unlock both

a.mu.Lock()

b := *a

a.mu.Unlock()

b.mu.Unlock()

Pro: fast and easy

Con: depends on copying a locked mutex results in a new locked mutex (currently true)

2) Copy it with mutex locked cand then replace the copied mutex:

a.mu.Lock()

b := *a

a.mu.Unlock()

b.mu = sync.Mutex{}

Pro: does not depend on a locked copied mutex staying locked

Con: still a dependency on mutex implementation, but not much of one

3) Copy each element one by one

var b TheStruct

b.a = a.a

b.b = a.b

...

Pro: does not copy the mutex

Con: have to copy each field and if TheStruct changes, this code must change.

In my case, I only had 3 fields aside from the mutex, so I went with option three.  If it had been bigger I would have gone with option 2.  Option 1 was what I first did, but then decided I didn't like the dependency that copying a locked mutex creates a new locked mutex.

   -Paul

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Dave Cheney]

But why did you need to clone the structure? That's the root cause, not the copying of the mutex.

[Paul Borman]

Because I needed a clone, and independent copy that could then be used independently of the first.  This would not work if they had pointers whose contents were protected by the mutex, but this was not the case.

One might ask why one would ever need fork or vfork :-)

    -Paul

[Manlio Perillo]

Il giorno venerdì 8 aprile 2016 17:18:13 UTC+2, Paul Borman ha scritto:

Because I needed a clone, and independent copy that could then be used independently of the first.  This would not work if they had pointers whose contents were protected by the mutex, but this was not the case.

One might ask why one would ever need fork or vfork :-)

Are you sure you really need a clone?

A clone operation will clone the internal state, and this causes a lot of troubles (e.g. mutex).

It is far more simple to create a new object with the same, externally visible, state.

Consider as an example log.Logger.

Instead of cloning the logger, you just create a new Logger with (assuming you have access to private data):

oldlog.mu.Lock()

newlog := logger.New(oldlog.out, oldlog.prefix,  oldlog.flag)

oldlog.mu.Unlock()

Note how Logger.mu and Logger.buf are not copied.

Manlio 

[Rob Pike]

It sounds like a mistake to me. Sometimes when you "need" something what's really needed is redesigning the code so the need doesn't arise.

What a lovely vacuous platitude. Sorry for that.

-rob

--

[roger peppe]

On 8 April 2016 at 15:42, 'Paul Borman' via golang-nuts

Another option might be to separate the mutex from the data that you
care about copying.

For example:

type data struct {
some data
}

type lockedData struct {
mu sync.Mutex
data
}

Then you can clone safely by doing:

func (d *lockedData) clone() *lockedData {
d.mu.Lock()
defer d.mu.Unlock()
return &lockedData{
data: d.data,
}
}

In practice you'd probably choose the nicer name for the struct
with the mutex, as it's likely to be the publicly visible type.

cheers,
rog.

[Paul Borman]

Manlio,

Logger is not an ideal example as you only want parts of it cloned (the buf field).  I would not try to clone the Logger structure with a copy since I don't want a clone in the end, only want some field duplicated.  As I mentioned, if you have a pointer whose contents are being guarded by the mutex, you would not want to copy the structure.

As for Rob, you don't have enough information to make that determination.  I have not advocated that one should be copying structures willy nilly, or even often.  I simply take issue with what was being said, that essentially there was never ever a situation in which you  would want to make a copy of a structure that had a mutex in it.  That is simply not true.  It may be infrequent, but that doesn't make it any less valid.

I encountered a legitimate need in the past few days.  I wanted a shallow copy of everything other than the mutex.  The mutex was not guarding any content that was being pointed to.  Simple example:

type Foo struct {

        // The following values do not change

        fd *os.File

        a  int

        // mu protects s, which can be changed

        mu sync.Mutex 

        s  string

}

  

The desire is to have a new Foo, just like the old one, but has an independent value of s, which has a setter, hence has a mutex.  Any of the three options I posited would suffice in creating func (f *Foo) Clone() *Foo, each with their own pros and cons.

Never say never, as you might be wrong.

    -Paul

[Manlio Perillo]

Il giorno venerdì 8 aprile 2016 18:21:06 UTC+2, Paul Borman ha scritto:

Manlio,

Logger is not an ideal example as you only want parts of it cloned (the buf field).  I would not try to clone the Logger structure with a copy since I don't want a clone in the end, only want some field duplicated.  As I mentioned, if you have a pointer whose contents are being guarded by the mutex, you would not want to copy the structure.

I suspect we are speaking about the same thing.

My point was that one usually does not want to clone a complex object, where with clone I intend a probably auto generated copy (deep or shallow) of all the internal state.

> [...]

  I wanted a shallow copy of everything other than the mutex.  The mutex was not guarding any content that was being pointed to.  Simple example:

type Foo struct {

        // The following values do not change

        fd *os.File

        a  int

        // mu protects s, which can be changed

        mu sync.Mutex 

        s  string

}

  

The desire is to have a new Foo, just like the old one, but has an independent value of s, which has a setter, hence has a mutex. 

I may be wrong, but isn't this precisely the same pattern I have used in my example?

The point is that I would not call this Clone, but Copy.

Clone has usually a well defined meaning, e.g.:

http://stackoverflow.com/questions/2427883/clone-vs-copy-constructor-which-is-recommended-in-java

Manlio 

[i...@liangsun.org]

What about type embedding? If put a sync.Mutex in a struct, that struct can not be embedded into another struct, right?

So I guess it's better idea to use structs with pointer to mutex.

[Ian Lance Taylor]

On Sun, Dec 11, 2016 at 11:48 PM, <i...@liangsun.org> wrote:
>
> What about type embedding? If put a sync.Mutex in a struct, that struct can
> not be embedded into another struct, right?

That turns out not to be the case. You can embed that struct in
another struct. But then, of course, you can not simply copy that
other struct.

> So I guess it's better idea to use structs with pointer to mutex.

It depends.

Ian

[i...@liangsun.org]

Consider this case:

package main

import (

"errors"

"fmt"

"sync"

)

type A struct {

Lock sync.Mutex

X    int

}

type B struct {

A

Y int

}

func newA(x int) (A, error) {

if x > 0 {

return A{}, errors.New("x should not be positive")

}

return A{X: x}, nil

}

func newB(x int, y int) (B, error) {

a, err := newA(x)

if err != nil {

return B{}, err

}

return B{

A: a,

Y: y,

}, nil

}

func main() {

b, err := newB(-1, 3)

fmt.Println(b.Y)

fmt.Println(err)

}

 

Actually, there is no way to avoid struct copy if the struct containing mutex is in another struct directly.

Run

go tool vet test.go

It will reports

test.go:32: literal copies lock value from a: main.A contains sync.Mutex

[Ian Lance Taylor]

You write as though you are disagreeing me, but as far as I can tell,
you are not.

Ian

> On Tuesday, December 13, 2016 at 3:16:14 AM UTC+8, Ian Lance Taylor wrote:
>>
>> On Sun, Dec 11, 2016 at 11:48 PM, <i...@liangsun.org> wrote:
>> >
>> > What about type embedding? If put a sync.Mutex in a struct, that struct
>> > can
>> > not be embedded into another struct, right?
>>
>> That turns out not to be the case. You can embed that struct in
>> another struct. But then, of course, you can not simply copy that
>> other struct.
>>
>> > So I guess it's better idea to use structs with pointer to mutex.
>>
>> It depends.
>>
>> Ian
>