Skip to content

Commit

Permalink
syncer(dm): fix the data race issue (#5881)
Browse files Browse the repository at this point in the history
close #4811
  • Loading branch information
lyzx2001 authored Jun 21, 2022
1 parent 8f3d664 commit 86780b1
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 1 deletion.
8 changes: 7 additions & 1 deletion dm/dm/worker/subtask.go
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,13 @@ func (st *SubTask) markResultCanceled() bool {
func (st *SubTask) Result() *pb.ProcessResult {
st.RLock()
defer st.RUnlock()
return st.result
if st.result == nil {
return nil
}
tempProcessResult, _ := st.result.Marshal()
newProcessResult := &pb.ProcessResult{}
_ = newProcessResult.Unmarshal(tempProcessResult)
return newProcessResult
}

// Close stops the sub task.
Expand Down
32 changes: 32 additions & 0 deletions dm/dm/worker/subtask_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -572,3 +572,35 @@ func TestValidatorStatus(t *testing.T) {
require.True(t, terror.ErrValidatorNotFound.Equal(err))
// validator != nil: will be tested in IT
}

func TestSubtaskRace(t *testing.T) {
// to test data race of Marshal() and markResultCanceled()
tempErrors := []*pb.ProcessError{}
tempDetail := []byte{}
tempProcessResult := pb.ProcessResult{
IsCanceled: false,
Errors: tempErrors,
Detail: tempDetail,
}
cfg := &config.SubTaskConfig{
Name: "test-subtask-race",
ValidatorCfg: config.ValidatorConfig{
Mode: config.ValidationFast,
},
}
st := NewSubTaskWithStage(cfg, pb.Stage_Paused, nil, "worker")
st.result = &tempProcessResult
tempQueryStatusResponse := pb.QueryStatusResponse{}
tempQueryStatusResponse.SubTaskStatus = make([]*pb.SubTaskStatus, 1)
tempSubTaskStatus := pb.SubTaskStatus{}
tempSubTaskStatus.Result = st.Result()
tempQueryStatusResponse.SubTaskStatus[0] = &tempSubTaskStatus
st.result.IsCanceled = false
go func() {
for i := 0; i < 10; i++ {
_, _ = tempQueryStatusResponse.Marshal()
}
}()
st.markResultCanceled()
// this test is to test data race, so don't need assert here
}

0 comments on commit 86780b1

Please sign in to comment.