Navigation Menu

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

Clean encode context before putting to pool #65

Closed
wants to merge 1 commit into from
Closed

Clean encode context before putting to pool #65

wants to merge 1 commit into from

Conversation

cuonglm
Copy link
Contributor

@cuonglm cuonglm commented Nov 24, 2020

Currently, once encode runtime context is done, we put it back to pool
to reuse later. The unsafe pointers in context keepRefs are also kept alive,
but its underlying object maybe cleaned already. So if the garbage
collector run, it will see an invalid pointer, the runtime crashes.

To fix it, we add encodeRuntimeContext.done method, to clean the keepRefs
once we finish using the context.

Fixes #63

Currently, once encode runtime context is done, we put it back to pool
to reuse later. The unsafe pointers in context keepRefs are also kept alive,
but its underlying object maybe cleaned already. So if the garbage
collector run, it will see an invalid pointer, the runtime crashes.

To fix it, we add encodeRuntimeContext.done method, to clean the keepRefs
once we finish using the context.

Fixes #63
@zchee
Copy link
Contributor

zchee commented Nov 24, 2020

@cuonglm Thanks!! this change passes GOGC=1 go1.15.5 test -v -count 1 -gcflags='all=-N -l' -run=. . .
However, still failed with -race actually:
GOGC=1 go1.15.5 test -v -race -count 1 -gcflags='all=-N -l' -run=. .

Do you have any idea about this?

$ GOGC=1 go1.15.5 test -v -race -count 1 -gcflags='all=-N -l' -run=. .
.
.
.
=== RUN   TestMarshalAllValue
runtime: pointer 0xc0002e5c30 to unallocated span span.base()=0xc0002de000 span.limit=0xc0002e6000 span.state=0
fatal error: found bad pointer in Go heap (incorrect use of unsafe or cgo?)

goroutine 51 [running]:
runtime.throw(0x14534c4, 0x3e)
	/Users/zchee/sdk/go1.15.5/src/runtime/panic.go:1116 +0x72 fp=0xc000389940 sp=0xc000389910 pc=0x107e432
runtime.badPointer(0x223ab98, 0xc0002e5c30, 0x0, 0x0)
	/Users/zchee/sdk/go1.15.5/src/runtime/mbitmap.go:380 +0x255 fp=0xc000389988 sp=0xc000389940 pc=0x1059a75
runtime.findObject(0xc0002e5c30, 0x0, 0x0, 0x14, 0x117297c, 0x1)
	/Users/zchee/sdk/go1.15.5/src/runtime/mbitmap.go:416 +0xb5 fp=0xc0003899d8 sp=0xc000389988 pc=0x1059b55
runtime.checkptrBase(0xc0002e5c30, 0xc000389a90)
	/Users/zchee/sdk/go1.15.5/src/runtime/checkptr.go:68 +0x4f fp=0xc000389a18 sp=0xc0003899d8 pc=0x104820f
runtime.checkptrAlignment(0xc0002e5c30, 0x13fd700, 0x1)
	/Users/zchee/sdk/go1.15.5/src/runtime/checkptr.go:19 +0x73 fp=0xc000389a50 sp=0xc000389a18 pc=0x1048033
reflect.Value.IsNil(0x14043e0, 0xc0002e5c30, 0x94, 0xc0002e5c00)
	/Users/zchee/sdk/go1.15.5/src/reflect/value.go:1084 +0x26d fp=0xc000389b20 sp=0xc000389a50 pc=0x116e2ad
github.com/goccy/go-json.(*Encoder).run(0xc0002b8070, 0xc0003113e0, 0xc000335a40, 0x0, 0x0)
	/Users/zchee/go/src/github.com/goccy/go-json/encode_vm.go:221 +0x89b fp=0xc00039f000 sp=0xc000389b20 pc=0x12f22db
github.com/goccy/go-json.(*Encoder).encode(0xc0002b8070, 0x143f360, 0xc00039fa38, 0x0, 0x0)
	/Users/zchee/go/src/github.com/goccy/go-json/encode.go:240 +0xc6a fp=0xc00039f338 sp=0xc00039f000 pc=0x12ad36a
github.com/goccy/go-json.(*Encoder).encodeForMarshal(0xc0002b8070, 0x143f360, 0xc000133a38, 0x0, 0x0, 0x0, 0x0, 0x0)
	/Users/zchee/go/src/github.com/goccy/go-json/encode.go:151 +0xb5 fp=0xc00039f4d0 sp=0xc00039f338 pc=0x12ac275
github.com/goccy/go-json.MarshalWithOption(0x143f360, 0xc000133a38, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
	/Users/zchee/go/src/github.com/goccy/go-json/json.go:169 +0x265 fp=0xc00039f628 sp=0xc00039f4d0 pc=0x136d225
github.com/goccy/go-json.Marshal(0x143f360, 0xc000077a38, 0x0, 0x0, 0x0, 0x0, 0x0)
	/Users/zchee/go/src/github.com/goccy/go-json/json.go:157 +0xe5 fp=0xc00039f708 sp=0xc00039f628 pc=0x136cec5
github.com/goccy/go-json_test.TestMarshalAllValue(0xc0002ba780)
	/Users/zchee/go/src/github.com/goccy/go-json/decode_test.go:1689 +0xe5 fp=0xc00039fe78 sp=0xc00039f708 pc=0x1386525
testing.tRunner(0xc0002ba780, 0x1455988)
	/Users/zchee/sdk/go1.15.5/src/testing/testing.go:1123 +0x2a5 fp=0xc00039ffd0 sp=0xc00039fe78 pc=0x11eb305
runtime.goexit()
	/Users/zchee/sdk/go1.15.5/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc00039ffd8 sp=0xc00039ffd0 pc=0x10b99a1
created by testing.(*T).Run
	/Users/zchee/sdk/go1.15.5/src/testing/testing.go:1168 +0xb07

goroutine 1 [chan receive]:
runtime.gopark(0x1455f80, 0xc000024e98, 0x11e170e, 0x2)
	/Users/zchee/sdk/go1.15.5/src/runtime/proc.go:306 +0xd5 fp=0xc000159290 sp=0xc000159260 pc=0x1081095
runtime.chanrecv(0xc000024e40, 0xc000159392, 0xc000000101, 0x11ebee7)
	/Users/zchee/sdk/go1.15.5/src/runtime/chan.go:577 +0x246 fp=0xc000159320 sp=0xc000159290 pc=0x10475a6
runtime.chanrecv1(0xc000024e40, 0xc000159392)
	/Users/zchee/sdk/go1.15.5/src/runtime/chan.go:439 +0x2b fp=0xc000159350 sp=0xc000159320 pc=0x104734b
testing.(*T).Run(0xc0002ba780, 0x1444d47, 0x13, 0x1455988, 0xd3c3eda2100)
	/Users/zchee/sdk/go1.15.5/src/testing/testing.go:1169 +0xb68 fp=0xc0001595d0 sp=0xc000159350 pc=0x11ebf48
testing.runTests.func1(0xc000102780)
	/Users/zchee/sdk/go1.15.5/src/testing/testing.go:1439 +0x186 fp=0xc0001596b0 sp=0xc0001595d0 pc=0x11f7a66
testing.tRunner(0xc000102780, 0xc000159a10)
	/Users/zchee/sdk/go1.15.5/src/testing/testing.go:1123 +0x2a5 fp=0xc000159808 sp=0xc0001596b0 pc=0x11eb305
testing.runTests(0xc000140a80, 0x1738860, 0x34, 0x34, 0xbfe75bfa323d57e8, 0x8bb2ec5976, 0x173e320, 0xc000120100)
	/Users/zchee/sdk/go1.15.5/src/testing/testing.go:1437 +0x7f1 fp=0xc000159a40 sp=0xc000159808 pc=0x11efc51
testing.(*M).Run(0xc00018e200, 0x0)
	/Users/zchee/sdk/go1.15.5/src/testing/testing.go:1345 +0x905 fp=0xc000159e90 sp=0xc000159a40 pc=0x11ed305
main.main()
	_testmain.go:175 +0x1bd fp=0xc000159f88 sp=0xc000159e90 pc=0x13e277d
runtime.main()
	/Users/zchee/sdk/go1.15.5/src/runtime/proc.go:204 +0x1cf fp=0xc000159fe0 sp=0xc000159f88 pc=0x1080c6f
runtime.goexit()
	/Users/zchee/sdk/go1.15.5/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc000159fe8 sp=0xc000159fe0 pc=0x10b99a1

goroutine 2 [force gc (idle)]:
runtime.gopark(0x1456270, 0x173df10, 0x1731411, 0x1)
	/Users/zchee/sdk/go1.15.5/src/runtime/proc.go:306 +0xd5 fp=0xc000064f88 sp=0xc000064f58 pc=0x1081095
runtime.goparkunlock(0x173df10, 0x1411, 0x1)
	/Users/zchee/sdk/go1.15.5/src/runtime/proc.go:312 +0x53 fp=0xc000064fb8 sp=0xc000064f88 pc=0x1081153
runtime.forcegchelper()
	/Users/zchee/sdk/go1.15.5/src/runtime/proc.go:255 +0xc5 fp=0xc000064fe0 sp=0xc000064fb8 pc=0x1080f05
runtime.goexit()
	/Users/zchee/sdk/go1.15.5/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc000064fe8 sp=0xc000064fe0 pc=0x10b99a1
created by runtime.init.6
	/Users/zchee/sdk/go1.15.5/src/runtime/proc.go:243 +0x35

goroutine 3 [GC sweep wait]:
runtime.gopark(0x1456270, 0x173e080, 0x106140c, 0x1)
	/Users/zchee/sdk/go1.15.5/src/runtime/proc.go:306 +0xd5 fp=0xc000065780 sp=0xc000065750 pc=0x1081095
runtime.goparkunlock(0x173e080, 0x15f140c, 0x1)
	/Users/zchee/sdk/go1.15.5/src/runtime/proc.go:312 +0x53 fp=0xc0000657b0 sp=0xc000065780 pc=0x1081153
runtime.bgsweep(0xc0000264d0)
	/Users/zchee/sdk/go1.15.5/src/runtime/mgcsweep.go:163 +0xa8 fp=0xc0000657d8 sp=0xc0000657b0 pc=0x1069968
runtime.goexit()
	/Users/zchee/sdk/go1.15.5/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc0000657e0 sp=0xc0000657d8 pc=0x10b99a1
created by runtime.gcenable
	/Users/zchee/sdk/go1.15.5/src/runtime/mgc.go:217 +0x5c

goroutine 4 [GC scavenge wait]:
runtime.gopark(0x1456270, 0x173e260, 0x106140d, 0x1)
	/Users/zchee/sdk/go1.15.5/src/runtime/proc.go:306 +0xd5 fp=0xc000065f50 sp=0xc000065f20 pc=0x1081095
runtime.goparkunlock(0x173e260, 0x15f140d, 0x1)
	/Users/zchee/sdk/go1.15.5/src/runtime/proc.go:312 +0x53 fp=0xc000065f80 sp=0xc000065f50 pc=0x1081153
runtime.bgscavenge(0xc0000264d0)
	/Users/zchee/sdk/go1.15.5/src/runtime/mgcscavenge.go:265 +0xdc fp=0xc000065fd8 sp=0xc000065f80 pc=0x106781c
runtime.goexit()
	/Users/zchee/sdk/go1.15.5/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc000065fe0 sp=0xc000065fd8 pc=0x10b99a1
created by runtime.gcenable
	/Users/zchee/sdk/go1.15.5/src/runtime/mgc.go:218 +0x7e

goroutine 18 [finalizer wait]:
runtime.gopark(0x1456270, 0x176dd28, 0x1761410, 0x1)
	/Users/zchee/sdk/go1.15.5/src/runtime/proc.go:306 +0xd5 fp=0xc000064728 sp=0xc0000646f8 pc=0x1081095
runtime.goparkunlock(0x176dd28, 0x1711410, 0x1)
	/Users/zchee/sdk/go1.15.5/src/runtime/proc.go:312 +0x53 fp=0xc000064758 sp=0xc000064728 pc=0x1081153
runtime.runfinq()
	/Users/zchee/sdk/go1.15.5/src/runtime/mfinal.go:175 +0x96 fp=0xc0000647e0 sp=0xc000064758 pc=0x105e856
runtime.goexit()
	/Users/zchee/sdk/go1.15.5/src/runtime/asm_amd64.s:1374 +0x1 fp=0xc0000647e8 sp=0xc0000647e0 pc=0x10b99a1
created by runtime.createfing
	/Users/zchee/sdk/go1.15.5/src/runtime/mfinal.go:156 +0x65
FAIL	github.com/goccy/go-json	0.064s
FAIL

@zchee
Copy link
Contributor

zchee commented Nov 24, 2020

this line:

if rv.IsNil() {

might be reflect.Value.ptr is bad pointer...?

@cuonglm
Copy link
Contributor Author

cuonglm commented Nov 24, 2020

Note that the error with -race happens even without GOGC=1. And also using all=-d=checkptr -N -l doesn't fail.

@cuonglm
Copy link
Contributor Author

cuonglm commented Nov 26, 2020

@zchee Close this as discussion in Slack.

@cuonglm cuonglm closed this Nov 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

found bad pointer in Go heap error
2 participants