search

Azure / azure-amqp

AMQP C# library
Other
94 stars 70 forks source link

ReceiveMessagesAsync can return null under heavy load #254

Closed JoshLove-msft closed 6 months ago

JoshLove-msft commented 8 months ago

ReceivingAmqpLink.ReceiveMessagesAsync can return null under heavy load. This was found due to the Azure.Messaging.ServiceBus library's ServiceBusReceiver.ReceiveMessagesAsync method occasionally throwing a NullReferenceException. This can be patched up in the Service Bus library, but it would affect all consuming libraries, so it should be fixed in the AMQP library.

danielmarbach commented 8 months ago

I have only glanced at the code a bit and did not spend much time on this so it might be incorrect. By looking briefly at the code it seems for efficiency reasons the ReceiveAsyncResult leaves the messages field null and lazy initializes it. This can lead to thisPtr.messages to return null. Master guards against that case already but the hotfix branches don't.

I have added a PR that could fix it. Unfortunately the hotfix branch is hard to get compiling on Linux which I run on so the PR might be of not so great quality.

What's also possible is that the caller of Signal with the messages list should actually hold a lock on the SyncRoot similar to what Add already assumes. Nonetheless guarding against the field not being initialized seems to be a good idea.