Concurrency issue on responseNotifyer map

[chris-skud]

Hi @qasaur,

First, thanks for your hard work on providing a Go Gremlin client. Really appreciate it. Let me know if i've misinterpreted the intended use of the package in any way... On to the issue.

Background: I first encountered the issue when performance test my API using ab (Apache Bench) with concurrency set to "2". I was not able to recreate using the existing test suite and therefore had to simulate concurrent "Execute" calls with a new test (see below).

tested on: go 1.7.3

Reproduce:

1. add the following code to client_test.go

func TestClient(t *testing.T) {
    dialer := NewDialer("127.0.0.1:8182")
    g, err := Dial(dialer)
    if err != nil {
        t.Fatal("Dial Error: ", err)
    }

    ch := make(chan bool)
    go func() {
        // first access
        g.Execute(
            "system.graph('foo').ifNotExists().create()",
            map[string]string{}, map[string]string{},
        )
        ch <- true
    }()

    // second access
    g.Execute(
        "system.graph('foo').ifNotExists().create()",
        map[string]string{}, map[string]string{},
    )
    <-ch
}

2. run go test -race -v

output:

=== RUN   TestClient
==================
WARNING: DATA RACE
Write at 0x00c420019080 by goroutine 9:
  runtime.mapassign1()
      /Users/skud/.gimme/versions/go1.7.3.darwin.amd64/src/runtime/hashmap.go:442 +0x0
  github.com/qasaur/gremgo.(*Client).executeRequest()
      /Users/skud/projects/src/github.com/qasaur/gremgo/client.go:61 +0x2e8
  github.com/qasaur/gremgo.(*Client).Execute()
      /Users/skud/projects/src/github.com/qasaur/gremgo/client.go:69 +0x70
  github.com/qasaur/gremgo.TestClient.func1()
      /Users/skud/projects/src/github.com/qasaur/gremgo/client_test.go:18 +0xc7

Previous write at 0x00c420019080 by goroutine 6:
  runtime.mapassign1()
      /Users/skud/.gimme/versions/go1.7.3.darwin.amd64/src/runtime/hashmap.go:442 +0x0
  github.com/qasaur/gremgo.(*Client).executeRequest()
      /Users/skud/projects/src/github.com/qasaur/gremgo/client.go:61 +0x2e8
  github.com/qasaur/gremgo.(*Client).Execute()
...

As indicated, the issue lies at: https://github.com/qasaur/gremgo/blob/master/client.go#L61

I believe you will also have issues at: https://github.com/qasaur/gremgo/blob/master/response.go#L65 and https://github.com/qasaur/gremgo/blob/master/response.go#L69.

A simple solution would be to add locks around those map ops, (perhaps consider RWMutex) but performance obviously may be affected.

[LeeWong]

Any update on this issue? Thanks.

[qasaur]

This issue has been fixed in https://github.com/qasaur/gremgo/pull/19.