-
Notifications
You must be signed in to change notification settings - Fork 4
Description
I can see a few issues with the implementation of the PriorityBufferBlock<T> class.
-
The
Postmethod doesn't propagate theboolreturn value of the underlying invocations. So if the three internal blocks have been configured withBoundedCapacityand the limit has been reached, the offered item will be declined, and the caller of thePriorityBufferBlock<T>.Postwill not be notified about it. The same is true for theSendAsyncmethod. -
Linking the
PriorityBufferBlock<T>to a target block with thePropagateCompletionenabled is broken. The completion will be propagated when the first of the three internal blocks completes, not when all complete. For example after callingPriorityBufferBlock<T>.Complete(), when the_highPriorityBufferis emptied it will trigger the completion of the target, so any messages that are still inside the_lowPriorityBufferwill be rejected by the target. These messages will not be consumed. -
The
_messagesByBlockis 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, theReserveMessage, 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 thePriorityBufferBlock<T>. When the internal blocks call theOfferMessageon the target, they are going to pass themselves as thesourceargument. So the target might call theReserveMessageon 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_messagesByBlockdictionary is a complexity that could be removed. TheConsumeMessage/ReserveMessage/ReleaseReservationmethods could just throw aNotSupportedException.