dotnet / dotnet/orleans

Dequeuing messages may cause unexpected NullReferenceException

Open
#7,738 2 comments 0 reactions 1 assignee Claimed by @benjaminpetit View on GitHub
area-streaming
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.