Skip to content

Commit

Permalink
Fix data race in Execute (#459)
Browse files Browse the repository at this point in the history
* Fix data race in Execute

* ensure goroutine doesn't hang

* fix rebase error
  • Loading branch information
egonelbre authored and Fontinalis committed Apr 2, 2019
1 parent 2b0b734 commit bed865f
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 31 deletions.
33 changes: 15 additions & 18 deletions executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,18 @@ func Execute(p ExecuteParams) (result *Result) {
addExtensionResults(&p, result)
}()

resultChannel := make(chan *Result)
result = &Result{
Extensions: map[string]interface{}{},
}
resultChannel := make(chan *Result, 2)

go func() {
result := &Result{}

go func(out chan<- *Result, done <-chan struct{}) {
defer func() {
if err := recover(); err != nil {
result.AppendErrors(gqlerrors.FormatError(err.(error)))
}
select {
case out <- result:
case <-done:
result.Errors = append(result.Errors, gqlerrors.FormatError(err.(error)))
}
resultChannel <- result
}()

exeContext, err := buildExecutionContext(buildExecutionCtxParams{
Schema: p.Schema,
Root: p.Root,
Expand All @@ -72,26 +69,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 {
Expand Down
2 changes: 1 addition & 1 deletion extensions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
12 changes: 0 additions & 12 deletions types.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package graphql

import (
"sync"

"github.com/graphql-go/graphql/gqlerrors"
)

Expand All @@ -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...)
}

0 comments on commit bed865f

Please sign in to comment.