-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Description
The block pruning functionality (activated by returning a RetainHeight from Commit) does not seem to be safe for concurrent use. One example of such a scenario appears to be triggerable when LoadBlock is called on the store.BlockStore, for example this part:
Lines 79 to 89 in 8e4c41e
| var blockMeta = bs.LoadBlockMeta(height) | |
| if blockMeta == nil { | |
| return nil | |
| } | |
| pbb := new(tmproto.Block) | |
| buf := []byte{} | |
| for i := 0; i < int(blockMeta.BlockID.PartSetHeader.Total); i++ { | |
| part := bs.LoadBlockPart(height, i) | |
| buf = append(buf, part.Bytes...) | |
| } |
If pruning is in progress (concurrently) then bs.LoadBlockPart can return nil as indicated by its documentation:
Line 128 in 8e4c41e
| // If no part is found for the given height and index, it returns nil. |
But since the nil value is not handled correctly this may result in a panic due to a nil dereference.
There seems to be a bs.mtx, but for some reason it is not locked when accessing state via Load* methods. This can result in the state changing while the Load* method is executing, e.g., in the above example the LoadBlockMeta can succeed while the subsequent LoadBlockPart will return nil as the block/part has since been removed.