Skip to content

Commit

Permalink
Fixes panic on sending errors to recordset. Resolves #67
Browse files Browse the repository at this point in the history
  • Loading branch information
khaf committed Jun 11, 2015
1 parent 63822c7 commit c156751
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 8 deletions.
22 changes: 14 additions & 8 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,9 +436,7 @@ func (clnt *Client) ScanAll(apolicy *ScanPolicy, namespace string, setName strin
for _, node := range nodes {
go func(node *Node) {
if err := clnt.scanNode(&policy, node, res, namespace, setName, binNames...); err != nil {
if _, ok := <-res.Errors; ok {
res.Errors <- err
}
res.sendError(err)
}
}(node)
}
Expand All @@ -447,9 +445,7 @@ func (clnt *Client) ScanAll(apolicy *ScanPolicy, namespace string, setName strin
go func() {
for _, node := range nodes {
if err := clnt.scanNode(&policy, node, res, namespace, setName, binNames...); err != nil {
if _, ok := <-res.Errors; ok {
res.Errors <- err
}
res.sendError(err)
continue
}
}
Expand Down Expand Up @@ -860,7 +856,12 @@ func (clnt *Client) Query(policy *QueryPolicy, statement *Statement) (*Recordset
// copy policies to avoid race conditions
newPolicy := *policy
command := newQueryRecordCommand(node, &newPolicy, statement, recSet)
go command.Execute()
go func() {
err := command.Execute()
if err != nil {
recSet.sendError(err)
}
}()
}

return recSet, nil
Expand Down Expand Up @@ -888,7 +889,12 @@ func (clnt *Client) QueryNode(policy *QueryPolicy, node *Node, statement *Statem
// copy policies to avoid race conditions
newPolicy := *policy
command := newQueryRecordCommand(node, &newPolicy, statement, recSet)
go command.Execute()
go func() {
err := command.Execute()
if err != nil {
recSet.sendError(err)
}
}()

return recSet, nil
}
Expand Down
12 changes: 12 additions & 0 deletions recordset.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ type Recordset struct {

active *AtomicBool
cancelled chan struct{}

chanLock sync.Mutex
}

// NewRecordset generates a new RecordSet instance.
Expand Down Expand Up @@ -115,6 +117,8 @@ func (rcs *Recordset) Close() {
// wait till all goroutines are done
rcs.wgGoroutines.Wait()

rcs.chanLock.Lock()
defer rcs.chanLock.Unlock()
close(rcs.Records)
close(rcs.Errors)
}
Expand All @@ -126,3 +130,11 @@ func (rcs *Recordset) signalEnd() {
rcs.Close()
}
}

func (rcs *Recordset) sendError(err error) {
if rcs.IsActive() {
rcs.chanLock.Lock()
defer rcs.chanLock.Unlock()
rcs.Errors <- err
}
}
39 changes: 39 additions & 0 deletions recordset_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright 2013-2015 Aerospike, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package aerospike

import (
"errors"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)

// ALL tests are isolated by SetName and Key, which are 50 random charachters
var _ = Describe("Recordset test", func() {

It("must avoid panic on sendError", func() {
rs := newRecordset(100, 1)

rs.sendError(errors.New("Error"))
rs.wgGoroutines.Done()
rs.Close()
rs.sendError(errors.New("Error"))

Expect(<-rs.Errors).NotTo(BeNil())
Expect(<-rs.Errors).To(BeNil())
})

})

0 comments on commit c156751

Please sign in to comment.