From 5a12f6f727e11e540145361a12c7af247edd5910 Mon Sep 17 00:00:00 2001 From: Egon Elbre Date: Sun, 10 Mar 2019 19:43:47 +0200 Subject: [PATCH 1/3] Fix data race in Execute --- executor.go | 30 +++++++++++++++--------------- types.go | 12 ------------ 2 files changed, 15 insertions(+), 27 deletions(-) diff --git a/executor.go b/executor.go index 93ec30e8..99fa7d87 100644 --- a/executor.go +++ b/executor.go @@ -47,20 +47,20 @@ func Execute(p ExecuteParams) (result *Result) { }() resultChannel := make(chan *Result) - result = &Result{ - Extensions: map[string]interface{}{}, - } - go func(out chan<- *Result, done <-chan struct{}) { + go func() { + result := &Result{} + defer func() { if err := recover(); err != nil { - result.AppendErrors(gqlerrors.FormatError(err.(error))) + result.Errors = append(result.Errors, gqlerrors.FormatError(err.(error))) } select { - case out <- result: - case <-done: + case resultChannel <- result: + case <-ctx.Done(): } }() + exeContext, err := buildExecutionContext(buildExecutionCtxParams{ Schema: p.Schema, Root: p.Root, @@ -72,26 +72,26 @@ func Execute(p ExecuteParams) (result *Result) { }) if err != nil { - result.AppendErrors(gqlerrors.FormatError(err)) + result.Errors = append(result.Errors, gqlerrors.FormatError(err.(error))) + resultChannel <- result return } - result = executeOperation(executeOperationParams{ + resultChannel <- executeOperation(executeOperationParams{ ExecutionContext: exeContext, Root: p.Root, Operation: exeContext.Operation, }) - - }(resultChannel, ctx.Done()) + }() select { case <-ctx.Done(): - result.AppendErrors(gqlerrors.FormatError(ctx.Err())) + result := &Result{} + result.Errors = append(result.Errors, gqlerrors.FormatError(ctx.Err())) + return result case r := <-resultChannel: - result = r + return r } - - return } type buildExecutionCtxParams struct { diff --git a/types.go b/types.go index b4a58047..5b991d8c 100644 --- a/types.go +++ b/types.go @@ -1,8 +1,6 @@ package graphql import ( - "sync" - "github.com/graphql-go/graphql/gqlerrors" ) @@ -13,19 +11,9 @@ type Result struct { Data interface{} `json:"data"` Errors []gqlerrors.FormattedError `json:"errors,omitempty"` Extensions map[string]interface{} `json:"extensions,omitempty"` - - errorsLock sync.Mutex } // HasErrors just a simple function to help you decide if the result has errors or not func (r *Result) HasErrors() bool { return len(r.Errors) > 0 } - -// AppendErrors is the thread-safe way to append error(s) to Result.Errors. -func (r *Result) AppendErrors(errs ...gqlerrors.FormattedError) { - r.errorsLock.Lock() - defer r.errorsLock.Unlock() - - r.Errors = append(r.Errors, errs...) -} From 552a1848c8cda7b4d3ca1601bfbade198e091083 Mon Sep 17 00:00:00 2001 From: Egon Elbre Date: Sun, 10 Mar 2019 19:55:33 +0200 Subject: [PATCH 2/3] ensure goroutine doesn't hang --- executor.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/executor.go b/executor.go index 99fa7d87..d8477140 100644 --- a/executor.go +++ b/executor.go @@ -46,7 +46,7 @@ func Execute(p ExecuteParams) (result *Result) { addExtensionResults(&p, result) }() - resultChannel := make(chan *Result) + resultChannel := make(chan *Result, 2) go func() { result := &Result{} @@ -55,10 +55,7 @@ func Execute(p ExecuteParams) (result *Result) { if err := recover(); err != nil { result.Errors = append(result.Errors, gqlerrors.FormatError(err.(error))) } - select { - case resultChannel <- result: - case <-ctx.Done(): - } + resultChannel <- result }() exeContext, err := buildExecutionContext(buildExecutionCtxParams{ From e637bd946abdbb18ab326f115aac423002213e8b Mon Sep 17 00:00:00 2001 From: Egon Elbre Date: Mon, 1 Apr 2019 13:21:03 +0300 Subject: [PATCH 3/3] fix rebase error --- extensions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions.go b/extensions.go index 8dd6f345..1c448fbf 100644 --- a/extensions.go +++ b/extensions.go @@ -236,7 +236,7 @@ func addExtensionResults(p *ExecuteParams, result *Result) { func() { defer func() { if r := recover(); r != nil { - result.AppendErrors(gqlerrors.FormatError(fmt.Errorf("%s.GetResult: %v", ext.Name(), r.(error)))) + result.Errors = append(result.Errors, gqlerrors.FormatError(fmt.Errorf("%s.GetResult: %v", ext.Name(), r.(error)))) } }() if ext.HasResult() {