Skip to content

Commit

Permalink
[CLIENT-2624] BatchGetOperate triggering SIGSEGV nil pointer in go cl…
Browse files Browse the repository at this point in the history
…ient

Caching of the operation is faulty and causes race conditions when used concurrently.
This commit removes the caching which included a usesless allocation and
rarely, if ever, had any practical influence on performance.
  • Loading branch information
khaf committed Oct 23, 2023
1 parent e8006b4 commit ee3d1e9
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 34 deletions.
9 changes: 0 additions & 9 deletions command.go
Original file line number Diff line number Diff line change
Expand Up @@ -2260,13 +2260,6 @@ func (cmd *baseCommand) writeBatchReadOperations(ops []*Operation, readAttr int)
func (cmd *baseCommand) writeOperationForOperation(operation *Operation) Error {
nameLength := copy(cmd.dataBuffer[(cmd.dataOffset+int(_OPERATION_HEADER_SIZE)):], operation.binName)

if operation.used {
// cahce will set the used flag to false again
if err := operation.cache(); err != nil {
return err
}
}

if operation.encoder == nil {
valueLength, err := operation.binValue.EstimateSize()
if err != nil {
Expand Down Expand Up @@ -2295,8 +2288,6 @@ func (cmd *baseCommand) writeOperationForOperation(operation *Operation) Error {
cmd.WriteByte((byte(nameLength)))
cmd.dataOffset += nameLength
_, err = operation.encoder(operation, cmd)
//mark the operation as used, so that it will be cached the next time it is used
operation.used = err == nil
return err
}

Expand Down
25 changes: 0 additions & 25 deletions operation.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,23 +115,12 @@ type Operation struct {

// will be true ONLY for GetHeader() operation
headerOnly bool

// reused determines if the operation is cached. If so, it will cache the
// internal bytes in binValue field and remove the encoder for maximum performance
used bool
}

// size returns the size of the operation on the wire protocol.
func (op *Operation) size() (int, Error) {
size := len(op.binName)

if op.used {
// cahce will set the used flag to false again
if err := op.cache(); err != nil {
return -1, err
}
}

// Simple case
if op.encoder == nil {
valueLength, err := op.binValue.EstimateSize()
Expand Down Expand Up @@ -162,20 +151,6 @@ func (op *Operation) grpc() *kvs.Operation {
}
}

// cache uses the encoder and caches the packed operation for further use.
func (op *Operation) cache() Error {
packer := newPacker()

if _, err := op.encoder(op, packer); err != nil {
return err
}

op.binValue = BytesValue(packer.Bytes())
op.encoder = nil // do not encode anymore; just use the cache
op.used = false // do not encode anymore; just use the cache
return nil
}

// GetBinOp creates read bin database operation.
func GetBinOp(binName string) *Operation {
return &Operation{opType: _READ, binName: binName, binValue: NewNullValue()}
Expand Down

0 comments on commit ee3d1e9

Please sign in to comment.