kevdever / tpldataflowhelpers_prioritybufferblock Goto Github PK

I can see a few issues with the implementation of the PriorityBufferBlock<T> class.

1. The Post method doesn't propagate the bool return value of the underlying invocations. So if the three internal blocks have been configured with BoundedCapacity and the limit has been reached, the offered item will be declined, and the caller of the PriorityBufferBlock<T>.Post will not be notified about it. The same is true for the SendAsync method.

2. Linking the PriorityBufferBlock<T> to a target block with the PropagateCompletion enabled is broken. The completion will be propagated when the first of the three internal blocks completes, not when all complete. For example after calling PriorityBufferBlock<T>.Complete(), when the _highPriorityBuffer is emptied it will trigger the completion of the target, so any messages that are still inside the _lowPriorityBuffer will be rejected by the target. These messages will not be consumed.

3. The _messagesByBlock is a dictionary where messages are only added, and never removed. So it can be a memory leak. My understanding is that the method that adds messages to the dictionary, the ReserveMessage, is never going to be called under normal circumstances. That's because the three internal blocks are linked to the target directly, and so the target is totally unaware of the existence of the PriorityBufferBlock<T>. When the internal blocks call the OfferMessage on the target, they are going to pass themselves as the source argument. So the target might call the ReserveMessage on the internal blocks, not on the wrapper class. If my understanding is correct, the aforementioned memory leak will never actually happen in practice, and the _messagesByBlock dictionary is a complexity that could be removed. The ConsumeMessage/ReserveMessage/ReleaseReservation methods could just throw a NotSupportedException.