From 032c8d5d8857439eb9ec52494d63802bb12dc872 Mon Sep 17 00:00:00 2001 From: liuq19 Date: Wed, 12 Jun 2024 17:29:26 +0800 Subject: [PATCH] test: enhance data race detection --- .github/workflows/unit_test-linux-x64.yml | 4 ++ internal/decoder/decode_norace.go | 73 +++++++++++++++++++++ internal/decoder/decode_race.go | 80 +++++++++++++++++++++++ internal/decoder/decoder.go | 47 ------------- internal/encoder/encode_norace.go | 44 +++++++++++++ internal/encoder/encode_race.go | 53 +++++++++++++++ internal/encoder/encoder.go | 18 ----- issue_test/race_test_go | 71 ++++++++++++++++++++ scripts/test_race.sh | 21 ++++++ 9 files changed, 346 insertions(+), 65 deletions(-) create mode 100644 internal/decoder/decode_norace.go create mode 100644 internal/decoder/decode_race.go create mode 100644 internal/encoder/encode_norace.go create mode 100644 internal/encoder/encode_race.go create mode 100644 issue_test/race_test_go create mode 100755 scripts/test_race.sh diff --git a/.github/workflows/unit_test-linux-x64.yml b/.github/workflows/unit_test-linux-x64.yml index 9931abb2c..2bfce166c 100644 --- a/.github/workflows/unit_test-linux-x64.yml +++ b/.github/workflows/unit_test-linux-x64.yml @@ -28,6 +28,10 @@ jobs: restore-keys: | ${{ runner.os }}-go- + - name: Data Race + run: | + ./scripts/test_race.sh + - name: Unit Test run: | go test -race -covermode=atomic -coverprofile=coverage.txt ./... diff --git a/internal/decoder/decode_norace.go b/internal/decoder/decode_norace.go new file mode 100644 index 000000000..6b1780920 --- /dev/null +++ b/internal/decoder/decode_norace.go @@ -0,0 +1,73 @@ +//go:build !race +// +build !race + +/* + * Copyright 2021 ByteDance Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package decoder + +import ( + `unsafe` + `encoding/json` + `reflect` + `runtime` + + `github.com/bytedance/sonic/internal/rt` + `github.com/bytedance/sonic/utf8` +) + +// Decode parses the JSON-encoded data from current position and stores the result +// in the value pointed to by val. +func (self *Decoder) Decode(val interface{}) error { + /* validate json if needed */ + if (self.f & (1 << _F_validate_string)) != 0 && !utf8.ValidateString(self.s){ + dbuf := utf8.CorrectWith(nil, rt.Str2Mem(self.s), "\ufffd") + self.s = rt.Mem2Str(dbuf) + } + + vv := rt.UnpackEface(val) + vp := vv.Value + + /* check for nil type */ + if vv.Type == nil { + return &json.InvalidUnmarshalError{} + } + + /* must be a non-nil pointer */ + if vp == nil || vv.Type.Kind() != reflect.Ptr { + return &json.InvalidUnmarshalError{Type: vv.Type.Pack()} + } + + etp := rt.PtrElem(vv.Type) + + /* check the defined pointer type for issue 379 */ + if vv.Type.IsNamed() { + newp := vp + etp = vv.Type + vp = unsafe.Pointer(&newp) + } + + /* create a new stack, and call the decoder */ + sb := newStack() + nb, err := decodeTypedPointer(self.s, self.i, etp, vp, sb, self.f) + /* return the stack back */ + self.i = nb + freeStack(sb) + + /* avoid GC ahead */ + runtime.KeepAlive(vv) + return err +} \ No newline at end of file diff --git a/internal/decoder/decode_race.go b/internal/decoder/decode_race.go new file mode 100644 index 000000000..695171920 --- /dev/null +++ b/internal/decoder/decode_race.go @@ -0,0 +1,80 @@ +//go:build race +// +build race + +/* + * Copyright 2021 ByteDance Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package decoder + +import ( + `unsafe` + `encoding/json` + `reflect` + `runtime` + + `github.com/bytedance/sonic/internal/rt` + `github.com/bytedance/sonic/utf8` +) + +func helpDetectDataRace(val interface{}) { + _, _ = json.Marshal(val) +} + +// Decode parses the JSON-encoded data from current position and stores the result +// in the value pointed to by val. +func (self *Decoder) Decode(val interface{}) error { + /* helper for detect data race when decode into prefilled values*/ + helpDetectDataRace(val) + + /* validate json if needed */ + if (self.f & (1 << _F_validate_string)) != 0 && !utf8.ValidateString(self.s){ + dbuf := utf8.CorrectWith(nil, rt.Str2Mem(self.s), "\ufffd") + self.s = rt.Mem2Str(dbuf) + } + + vv := rt.UnpackEface(val) + vp := vv.Value + + /* check for nil type */ + if vv.Type == nil { + return &json.InvalidUnmarshalError{} + } + + /* must be a non-nil pointer */ + if vp == nil || vv.Type.Kind() != reflect.Ptr { + return &json.InvalidUnmarshalError{Type: vv.Type.Pack()} + } + + etp := rt.PtrElem(vv.Type) + + /* check the defined pointer type for issue 379 */ + if vv.Type.IsNamed() { + newp := vp + etp = vv.Type + vp = unsafe.Pointer(&newp) + } + + /* create a new stack, and call the decoder */ + sb := newStack() + nb, err := decodeTypedPointer(self.s, self.i, etp, vp, sb, self.f) + /* return the stack back */ + self.i = nb + freeStack(sb) + + /* avoid GC ahead */ + runtime.KeepAlive(vv) + return err +} \ No newline at end of file diff --git a/internal/decoder/decoder.go b/internal/decoder/decoder.go index 8453db861..4d5be4834 100644 --- a/internal/decoder/decoder.go +++ b/internal/decoder/decoder.go @@ -17,16 +17,12 @@ package decoder import ( - `unsafe` - `encoding/json` `reflect` - `runtime` `github.com/bytedance/sonic/internal/native` `github.com/bytedance/sonic/internal/native/types` `github.com/bytedance/sonic/internal/rt` `github.com/bytedance/sonic/option` - `github.com/bytedance/sonic/utf8` ) const ( @@ -106,49 +102,6 @@ func (self *Decoder) CheckTrailings() error { } -// Decode parses the JSON-encoded data from current position and stores the result -// in the value pointed to by val. -func (self *Decoder) Decode(val interface{}) error { - /* validate json if needed */ - if (self.f & (1 << _F_validate_string)) != 0 && !utf8.ValidateString(self.s){ - dbuf := utf8.CorrectWith(nil, rt.Str2Mem(self.s), "\ufffd") - self.s = rt.Mem2Str(dbuf) - } - - vv := rt.UnpackEface(val) - vp := vv.Value - - /* check for nil type */ - if vv.Type == nil { - return &json.InvalidUnmarshalError{} - } - - /* must be a non-nil pointer */ - if vp == nil || vv.Type.Kind() != reflect.Ptr { - return &json.InvalidUnmarshalError{Type: vv.Type.Pack()} - } - - etp := rt.PtrElem(vv.Type) - - /* check the defined pointer type for issue 379 */ - if vv.Type.IsNamed() { - newp := vp - etp = vv.Type - vp = unsafe.Pointer(&newp) - } - - /* create a new stack, and call the decoder */ - sb := newStack() - nb, err := decodeTypedPointer(self.s, self.i, etp, vp, sb, self.f) - /* return the stack back */ - self.i = nb - freeStack(sb) - - /* avoid GC ahead */ - runtime.KeepAlive(vv) - return err -} - // UseInt64 indicates the Decoder to unmarshal an integer into an interface{} as an // int64 instead of as a float64. func (self *Decoder) UseInt64() { diff --git a/internal/encoder/encode_norace.go b/internal/encoder/encode_norace.go new file mode 100644 index 000000000..965f341d4 --- /dev/null +++ b/internal/encoder/encode_norace.go @@ -0,0 +1,44 @@ +//go:build !race +// +build !race + +/* + * Copyright 2021 ByteDance Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package encoder + +import ( + `runtime` + + `github.com/bytedance/sonic/internal/rt` +) + + +func encodeInto(buf *[]byte, val interface{}, opts Options) error { + stk := newStack() + efv := rt.UnpackEface(val) + err := encodeTypedPointer(buf, efv.Type, &efv.Value, stk, uint64(opts)) + + /* return the stack into pool */ + if err != nil { + resetStack(stk) + } + freeStack(stk) + + /* avoid GC ahead */ + runtime.KeepAlive(buf) + runtime.KeepAlive(efv) + return err +} \ No newline at end of file diff --git a/internal/encoder/encode_race.go b/internal/encoder/encode_race.go new file mode 100644 index 000000000..27152c560 --- /dev/null +++ b/internal/encoder/encode_race.go @@ -0,0 +1,53 @@ + +//go:build race +// +build race + +/* + * Copyright 2021 ByteDance Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package encoder + +import ( + `encoding/json` + `runtime` + + `github.com/bytedance/sonic/internal/rt` +) + + +func helpDetectDataRace(val interface{}) { + _, _ = json.Marshal(val) +} + +func encodeInto(buf *[]byte, val interface{}, opts Options) error { + /* helper for detect data race */ + helpDetectDataRace(val) + + stk := newStack() + efv := rt.UnpackEface(val) + err := encodeTypedPointer(buf, efv.Type, &efv.Value, stk, uint64(opts)) + + /* return the stack into pool */ + if err != nil { + resetStack(stk) + } + freeStack(stk) + + /* avoid GC ahead */ + runtime.KeepAlive(buf) + runtime.KeepAlive(efv) + return err +} \ No newline at end of file diff --git a/internal/encoder/encoder.go b/internal/encoder/encoder.go index d285c2991..df90b1050 100644 --- a/internal/encoder/encoder.go +++ b/internal/encoder/encoder.go @@ -20,7 +20,6 @@ import ( `bytes` `encoding/json` `reflect` - `runtime` `unsafe` `github.com/bytedance/sonic/internal/native` @@ -233,23 +232,6 @@ func EncodeInto(buf *[]byte, val interface{}, opts Options) error { return err } -func encodeInto(buf *[]byte, val interface{}, opts Options) error { - stk := newStack() - efv := rt.UnpackEface(val) - err := encodeTypedPointer(buf, efv.Type, &efv.Value, stk, uint64(opts)) - - /* return the stack into pool */ - if err != nil { - resetStack(stk) - } - freeStack(stk) - - /* avoid GC ahead */ - runtime.KeepAlive(buf) - runtime.KeepAlive(efv) - return err -} - func encodeFinish(buf []byte, opts Options) []byte { if opts & EscapeHTML != 0 { buf = HTMLEscape(nil, buf) diff --git a/issue_test/race_test_go b/issue_test/race_test_go new file mode 100644 index 000000000..52449f4e1 --- /dev/null +++ b/issue_test/race_test_go @@ -0,0 +1,71 @@ + +//go:build race +// +build race + +/* + * Copyright 2024 ByteDance Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + +package issue_test + +import ( + `github.com/bytedance/sonic` + `testing` +) + +type MyFoo struct { + List []*int64 +} + +func TestRaceEncode(t *testing.T) { + f := &MyFoo{ + List: []*int64{new(int64), new(int64)}, + } + + go func() { + f.List = []*int64{new(int64), new(int64)} + }() + + // go func() { + // sonic.Marshal(f) + // }() + + // encoding/json always detect data race when enabling `-race` here + // go func() { + // json.Marshal(f) + // }() +} + + +func TestRaceDecode(t *testing.T) { + f := &MyFoo{ + List: []*int64{new(int64), new(int64), new(int64)}, + } + + i := int64(200) + go func() { + f.List[2] = &i + }() + + go func() { + sonic.Unmarshal([]byte(`{"List":[1,2,3]}`), f) + }() + + // encoding/json always detect data race when enabling `-race` here + // go func() { + // json.Unmarshal([]byte(`{"List":[1,2,3]}`), f) + // }() +} \ No newline at end of file diff --git a/scripts/test_race.sh b/scripts/test_race.sh new file mode 100755 index 000000000..f7ddd133c --- /dev/null +++ b/scripts/test_race.sh @@ -0,0 +1,21 @@ +#!/bin/bash + +set -xe + +mv ./issue_test/race_test_go ./issue_test/race_test.go + +go test -v -run=TestRaceEncode -race -count=100 ./issue_test > test_race.log || true + +if ! grep -q "WARNING: DATA RACE" ./test_race.log; then + echo "TEST FAILED: should data race here" + exit 1 +fi + +go test -v -run=TestRaceDecode -race -count=100 ./issue_test > test_race.log || true + +if ! grep -q "WARNING: DATA RACE" ./test_race.log; then + echo "TEST FAILED: should data race here" + exit 1 +fi + +mv ./issue_test/race_test.go ./issue_test/race_test_go \ No newline at end of file