Fix race on ApplyTo

The FSM tries to decode an operation on top of the
*LogOp. We might still be using the *LogOp.Cid.Allocations
slice. We need to make a deep of *LogOp.Cid before
returning from ApplyTo.

This one was tricky...

License: MIT
Signed-off-by: Hector Sanjuan <code@hector.link>
This commit is contained in:
Hector Sanjuan 2018-10-27 00:40:06 +02:00
parent 765987ef2c
commit d63a5e2667
2 changed files with 22 additions and 4 deletions

View File

@ -829,6 +829,15 @@ func (pins PinSerial) ToPin() Pin {
}
}
// Clone returns a deep copy of the PinSerial.
func (pins PinSerial) Clone() PinSerial {
new := pins // this copy all the simple fields.
// slices are pointers. We need to explicitally copy them.
new.Allocations = make([]string, len(pins.Allocations))
copy(new.Allocations, pins.Allocations)
return new
}
// NodeWithMeta specifies a block of data and a set of optional metadata fields
// carrying information about the encoded ipld node
type NodeWithMeta struct {

View File

@ -36,9 +36,18 @@ func (op *LogOp) ApplyTo(cstate consensus.State) (consensus.State, error) {
panic("received unexpected state type")
}
// Copy the Cid. We are about to pass it to go-routines
// that will make things with it (read its fields). However,
// as soon as ApplyTo is done, the next operation will be deserealized
// on top of "op". This can cause data races with the slices in
// api.PinSerial, which don't get copied when passed.
pinS := op.Cid.Clone()
pin := pinS.ToPin()
switch op.Type {
case LogOpPin:
err = state.Add(op.Cid.ToPin())
err = state.Add(pin)
if err != nil {
goto ROLLBACK
}
@ -46,11 +55,11 @@ func (op *LogOp) ApplyTo(cstate consensus.State) (consensus.State, error) {
op.consensus.rpcClient.Go("",
"Cluster",
"Track",
op.Cid,
pinS,
&struct{}{},
nil)
case LogOpUnpin:
err = state.Rm(op.Cid.ToPin().Cid)
err = state.Rm(pin.Cid)
if err != nil {
goto ROLLBACK
}
@ -58,7 +67,7 @@ func (op *LogOp) ApplyTo(cstate consensus.State) (consensus.State, error) {
op.consensus.rpcClient.Go("",
"Cluster",
"Untrack",
op.Cid,
pinS,
&struct{}{},
nil)
default: