Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data method is not thread safe. #13

Closed
e-max opened this issue Jan 25, 2016 · 10 comments
Closed

Data method is not thread safe. #13

e-max opened this issue Jan 25, 2016 · 10 comments
Labels

Comments

@e-max
Copy link
Contributor

e-max commented Jan 25, 2016

Though this method is protected by lock and create a new structure on every call, this structure still use the same instance of ResponseCounts. So it is possible that somebody would read this map in the same moment as EndWithStatus would write into it.
In go 1.6 it will cause a panic.

@thoas
Copy link
Owner

thoas commented Mar 15, 2016

could you add some tests on this?

e-max added a commit to e-max/stats that referenced this issue Mar 15, 2016
@e-max
Copy link
Contributor Author

e-max commented Mar 15, 2016

I am not sure how to make this tests not so ugly )
#14

@e-max
Copy link
Contributor Author

e-max commented Mar 15, 2016

It cause panic only if use go 1.6. I am not sure how to test it on previous versions.

@heu
Copy link

heu commented Apr 21, 2016

It seem that the returns of Data() function include the maps (references) from Stats structure, any further access to those maps is not protected by RWMutex (privated in Stats)

@emilevauge
Copy link

Hi!
Any news on this?
I'm also having this issue on traefik traefik/traefik#458.
We are getting this error while json encoding the Data struct:

fatal error: concurrent map read and map write

goroutine 6569 [running]:
runtime.throw(0x10932c0, 0x21)
        /usr/local/go/src/runtime/panic.go:547 +0x90 fp=0xc8213b1128 sp=0xc8213b1110
runtime.mapaccess2(0xc27f20, 0xc8202a99b0, 0xc82000bb10, 0x3, 0x6c7d9e)
        /usr/local/go/src/runtime/hashmap.go:343 +0x5a fp=0xc8213b1170 sp=0xc8213b1128
reflect.mapaccess(0xc27f20, 0xc8202a99b0, 0xc82000bb10, 0xc8202a99b0)
        /usr/local/go/src/runtime/hashmap.go:993 +0x35 fp=0xc8213b11a0 sp=0xc8213b1170
reflect.Value.MapIndex(0xc27f20, 0xc830c04160, 0x195, 0xc2caa0, 0xc82000bb10, 0x98, 0x0, 0x0, 0x0)
        /usr/local/go/src/reflect/value.go:1041 +0x14a fp=0xc8213b1228 sp=0xc8213b11a0
encoding/json.(*mapEncoder).encode(0xc825a10310, 0xc8306c00b0, 0xc27f20, 0xc830c04160, 0x195, 0x0)
        /usr/local/go/src/encoding/json/encode.go:622 +0x2ec fp=0xc8213b1318 sp=0xc8213b1228
encoding/json.(*mapEncoder).(encoding/json.encode)-fm(0xc8306c00b0, 0xc27f20, 0xc830c04160, 0x195, 0x0)
        /usr/local/go/src/encoding/json/encode.go:632 +0x51 fp=0xc8213b1350 sp=0xc8213b1318
encoding/json.(*structEncoder).encode(0xc8258dcb40, 0xc8306c00b0, 0xf2a9a0, 0xc830c04120, 0x199, 0x0)
        /usr/local/go/src/encoding/json/encode.go:587 +0x2c4 fp=0xc8213b14f8 sp=0xc8213b1350
encoding/json.(*structEncoder).(encoding/json.encode)-fm(0xc8306c00b0, 0xf2a9a0, 0xc830c04120, 0x199, 0xc830c04100)
        /usr/local/go/src/encoding/json/encode.go:601 +0x51 fp=0xc8213b1530 sp=0xc8213b14f8
encoding/json.(*ptrEncoder).encode(0xc825a10350, 0xc8306c00b0, 0xbd76a0, 0xc830c04120, 0x16, 0x0)
        /usr/local/go/src/encoding/json/encode.go:709 +0xea fp=0xc8213b1580 sp=0xc8213b1530
encoding/json.(*ptrEncoder).(encoding/json.encode)-fm(0xc8306c00b0, 0xbd76a0, 0xc830c04120, 0x16, 0xc830c04100)
        /usr/local/go/src/encoding/json/encode.go:714 +0x51 fp=0xc8213b15b8 sp=0xc8213b1580
encoding/json.(*encodeState).reflectValue(0xc8306c00b0, 0xbd76a0, 0xc830c04120, 0x16)
        /usr/local/go/src/encoding/json/encode.go:301 +0x6b fp=0xc8213b15e8 sp=0xc8213b15b8
encoding/json.(*encodeState).marshal(0xc8306c00b0, 0xbd76a0, 0xc830c04120, 0x0, 0x0)
        /usr/local/go/src/encoding/json/encode.go:274 +0xa9 fp=0xc8213b1630 sp=0xc8213b15e8
encoding/json.Marshal(0xbd76a0, 0xc830c04120, 0x0, 0x0, 0x0, 0x0, 0x0)

@emilevauge
Copy link

Ping @thoas :)

@thoas
Copy link
Owner

thoas commented Jul 18, 2016

will look into this, you can send me a PR if you want :)

thoas added a commit that referenced this issue Jul 18, 2016
@thoas
Copy link
Owner

thoas commented Jul 18, 2016

fixed

@thoas thoas closed this as completed Jul 18, 2016
@thoas thoas reopened this Jul 18, 2016
@thoas
Copy link
Owner

thoas commented Jul 18, 2016

reopening still not fixed

@thoas thoas added the bug label Jul 18, 2016
@thoas
Copy link
Owner

thoas commented Jul 26, 2016

fixed in master

@thoas thoas closed this as completed Jul 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants