From f72bbb9cc458a69405cca2f41fe9c7ff54047a06 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 2 Feb 2021 18:48:41 +0100 Subject: [PATCH 1/5] Add race conditions into test --- modules/process/manager_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/process/manager_test.go b/modules/process/manager_test.go index 42f4b0c04b5a1..4180f6ee9b935 100644 --- a/modules/process/manager_test.go +++ b/modules/process/manager_test.go @@ -13,7 +13,10 @@ import ( ) func TestManager_Add(t *testing.T) { - pm := Manager{processes: make(map[int64]*Process)} + go func() { + _ = GetManager() + }() + pm := GetManager() pid := pm.Add("foo", nil) assert.Equal(t, int64(1), pid, "expected to get pid 1 got %d", pid) From 1cce40f70e7a8615215fca6ba92b5644e13d5e38 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 2 Feb 2021 18:52:41 +0100 Subject: [PATCH 2/5] Fix Race in GetManager() --- modules/process/manager.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/process/manager.go b/modules/process/manager.go index 27ed1d4964d15..9d57f4eb7b903 100644 --- a/modules/process/manager.go +++ b/modules/process/manager.go @@ -25,6 +25,7 @@ var ( // ErrExecTimeout represent a timeout error ErrExecTimeout = errors.New("Process execution timeout") manager *Manager + managerInit sync.Once // DefaultContext is the default context to run processing commands in DefaultContext = context.Background() @@ -48,11 +49,11 @@ type Manager struct { // GetManager returns a Manager and initializes one as singleton if there's none yet func GetManager() *Manager { - if manager == nil { + managerInit.Do(func() { manager = &Manager{ processes: make(map[int64]*Process), } - } + }) return manager } From 71d955034287ea4a281001f0897517ecc8947dfa Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 2 Feb 2021 19:18:34 +0100 Subject: [PATCH 3/5] DataAsync() use error chan --- modules/git/blob_nogogit.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/modules/git/blob_nogogit.go b/modules/git/blob_nogogit.go index 401b172860ee6..e870549cd3ae2 100644 --- a/modules/git/blob_nogogit.go +++ b/modules/git/blob_nogogit.go @@ -27,13 +27,14 @@ type Blob struct { // Calling the Close function on the result will discard all unread output. func (b *Blob) DataAsync() (io.ReadCloser, error) { stdoutReader, stdoutWriter := io.Pipe() - var err error + errChan := make(chan error) go func() { stderr := &strings.Builder{} - err = NewCommand("cat-file", "--batch").RunInDirFullPipeline(b.repoPath, stdoutWriter, stderr, strings.NewReader(b.ID.String()+"\n")) + err := NewCommand("cat-file", "--batch").RunInDirFullPipeline(b.repoPath, stdoutWriter, stderr, strings.NewReader(b.ID.String()+"\n")) if err != nil { err = ConcatenateError(err, stderr.String()) + errChan <- err _ = stdoutWriter.CloseWithError(err) } else { _ = stdoutWriter.Close() @@ -50,8 +51,8 @@ func (b *Blob) DataAsync() (io.ReadCloser, error) { return &LimitedReaderCloser{ R: bufReader, C: stdoutReader, - N: int64(size), - }, err + N: size, + }, <-errChan } // Size returns the uncompressed size of the blob From 927d91e6b114c99cbd095c0a92cf2fc6fdad38e7 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 2 Feb 2021 20:09:17 +0100 Subject: [PATCH 4/5] just log no chan --- modules/git/blob_nogogit.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/modules/git/blob_nogogit.go b/modules/git/blob_nogogit.go index e870549cd3ae2..731f7d06e8a60 100644 --- a/modules/git/blob_nogogit.go +++ b/modules/git/blob_nogogit.go @@ -11,6 +11,8 @@ import ( "io" "strconv" "strings" + + gitea_log "code.gitea.io/gitea/modules/log" ) // Blob represents a Git object. @@ -27,14 +29,13 @@ type Blob struct { // Calling the Close function on the result will discard all unread output. func (b *Blob) DataAsync() (io.ReadCloser, error) { stdoutReader, stdoutWriter := io.Pipe() - errChan := make(chan error) go func() { stderr := &strings.Builder{} err := NewCommand("cat-file", "--batch").RunInDirFullPipeline(b.repoPath, stdoutWriter, stderr, strings.NewReader(b.ID.String()+"\n")) if err != nil { err = ConcatenateError(err, stderr.String()) - errChan <- err + gitea_log.Error("Blob.DataAsync Error: %v", err) _ = stdoutWriter.CloseWithError(err) } else { _ = stdoutWriter.Close() @@ -52,7 +53,7 @@ func (b *Blob) DataAsync() (io.ReadCloser, error) { R: bufReader, C: stdoutReader, N: size, - }, <-errChan + }, nil } // Size returns the uncompressed size of the blob From 4d1f761bb1f906ca0ac11afd8bc7c0d57fbb2e0d Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 3 Feb 2021 19:35:32 +0100 Subject: [PATCH 5/5] finish --- modules/process/manager_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/modules/process/manager_test.go b/modules/process/manager_test.go index 4180f6ee9b935..a515fc32cda71 100644 --- a/modules/process/manager_test.go +++ b/modules/process/manager_test.go @@ -12,11 +12,17 @@ import ( "github.com/stretchr/testify/assert" ) -func TestManager_Add(t *testing.T) { +func TestGetManager(t *testing.T) { go func() { + // test race protection _ = GetManager() }() pm := GetManager() + assert.NotNil(t, pm) +} + +func TestManager_Add(t *testing.T) { + pm := Manager{processes: make(map[int64]*Process)} pid := pm.Add("foo", nil) assert.Equal(t, int64(1), pid, "expected to get pid 1 got %d", pid)