From ee3d1e90090f79060c73d41e0882019f10ece3fb Mon Sep 17 00:00:00 2001 From: Khosrow Afroozeh Date: Mon, 23 Oct 2023 14:52:10 +0200 Subject: [PATCH] [CLIENT-2624] BatchGetOperate triggering SIGSEGV nil pointer in go client 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. --- command.go | 9 --------- operation.go | 25 ------------------------- 2 files changed, 34 deletions(-) diff --git a/command.go b/command.go index 10b4b719..1f80fbd3 100644 --- a/command.go +++ b/command.go @@ -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 { @@ -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 } diff --git a/operation.go b/operation.go index d65f3fbe..4e147333 100644 --- a/operation.go +++ b/operation.go @@ -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() @@ -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()}