Dequeuing messages may cause unexpected NullReferenceException
- Dominant language
- C#
- Stars
- 10.9k
- Forks
- 2.1k
- Avg merge
- 13h 56m
- Merged PRs (30d)
- 351
Description
### Describe the bug
When using [`GetQueueMessage`](https://github.com/dotnet/orleans/blob/5ec98fb485ea89be192947878216a546f02e35d4/src/Azure/Orleans.Streaming.AzureStorage/Storage/AzureQueueDataManager.cs#L217-L238) method of `AzureQueueDataManager` to pop up a message from the queue based on Azure Queue service, it may pop up messages multiple times silently. Since Azure SDK API [`ReceiveMessagesAsync`](https://github.com/dotnet/orleans/blob/5ec98fb485ea89be192947878216a546f02e35d4/src/Azure/Orleans.Streaming.AzureStorage/Storage/AzureQueueDataManager.cs#L226) called inside `GetQueueMessage` has a retry mechanism when transient network errors (e.g., timeout) happen, a retry request would get and pop up the queue message for a second time, and the message from the first request would be invisible for a while. In this regard, the queue message we get may not be what is needed. Worse still, it is possible to make the queue empty due to retries so that the application will retrieve a null value from the queue. Later, when it refers to this null value of queue message, an unanticipated `System.NullReferenceException` would be thrown.
### To Reproduce
We found test [AQ_Standalone_1](https://github.com/dotnet/orleans/blob/5ec98fb485ea89be192947878216a546f02e35d4/test/Extensions/TesterAzureUtils/AzureQueueDataManagerTests.cs#L65-L67) could fail due to this bug when referring to the null value got from `GetQueueMessage`.
```
...
QueueMessage outMessage3 = await manager.GetQueueMessage(); // outMessage3 is NULL
logger.Info("GetQueueMessage 3: {0}", PrintQueueMessage(outMessage3)); // fail here with NullReferenceException
Assert.Equal(inMessage, outMessage3.MessageText);
...
```
### Discussion
When `GetQueueMessage` is called, it should not dequeue messages repeatedly without the application's awareness. We may want to inform the unexpected context changing of the queue to users like logging this change down. For instance, before calling `ReceiveMessagesAsync`, we can get the number of queue first. After `ReceiveMessagesAsync` finishes, we check the queue size again to see whether additional messages have been removed.
Contributor guide
Assessment
This issue has not been assessed yet.