Skip to content

ExecutionContext class is not thread safe #263

@bbakerman

Description

@bbakerman

The current implementation of ExecutionContext use a very mutable pattern and hence is not thread safe in Java. There is not "happens before" relationship in place with the various members.

public class ExecutionContext {

    private GraphQLSchema graphQLSchema;
    private ExecutionStrategy executionStrategy;
    private Map<String, FragmentDefinition> fragmentsByName = new LinkedHashMap<String, FragmentDefinition>();
    private OperationDefinition operationDefinition;
    private Map<String, Object> variables = new LinkedHashMap<String, Object>();
    private Object root;
    private List<GraphQLError> errors = new ArrayList<GraphQLError>();

It is especially evident in the errors list which is meant to be a running list of problems along the way. Testing with a ExecutorService reveals that errors MAY NOT be thread visible since its not using synchrnonised or better concurrent constructs. A CopyOnWriteAArrayList would help

Or the ExecutionContext could be made immutable. It would copy itself every time it needed to mutate (eg adding errors)

ExecutionContext newCtx = currentCtx.addError(someError);

Looking at the code it seems that ExecutionContextBuilder is the only setter of the attributes other than errors and hence I think the set calls could be removed and a CopyOnWriteAArrayList used to capture errors

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions