[BUG] [Microsoft.Azure.ServiceBus] Closing MessageReceiver Does not Always Close inner ReceivingAmqpLink

[paulsavides]

Describe the bug
During periods when a large number of server side errors occur, we sometimes see messages getting "stuck" in queues. As in, they hang in the queues for the configured message lock timeout before being redelivered.

After some attempts to reproduce with a smaller example, I have found that, in certain scenarios, when calling MessageReceiver.CloseAsync(), the inner ReceivingAmqpLink is not actually closed. So, the link is sitting there in the background, continually picking up messages

The only way I was able to get this to reproduce is by closing & opening a new receiver when receiving an error on the ExceptionHandler. My best guess to why this issue occurs; when the inner link faults, it will auto-recover in OnReceiveAsync(). Perhaps there is a race condition with auto recovery & closing the receiver at similar times.

Of course, perhaps there is something completely off with the usage of the sdk here as well.

Expected behavior
In general, I would expect that CloseAsync() would always close the inner ReceivingAmqpLink.

To Reproduce
Reproduction Repo = https://github.com/paulsavides/ServiceBusTesting

ReproProject is the project that reproduces this issue. If the code is doing something extremely incorrect, please let me know. We are actually using the MassTransit library to interact with AzureServiceBus so I had to recreate a bit of what it was doing that reproduces the error.

1. Open solution from production repo
2. Set ReproProject as Startup project
3. Fill in Endpoint & Shared Access Key Signature in Program.cs
4. Run the project
5. While the project is running, open the Queue in the Azure UI & continually update the Auto-delete after idle setting.
   • This is an attempt to cause errors that requires the links to be recreated
   • You can view this video to see exactly what I mean if the instructions are unclear https://www.youtube.com/watch?v=sv0bRozEevs
6. Eventually, in the console output, you will see errors coming through the exception handler & the receiver will 'recycle' some number of times
7. After recycle, you should start seeing message sends & receives being mismatched
   • if not, go back to step 5
8. Press d to print out diagnostics on all of the links from "closed" receivers that are still open & the number of unsettled messages from those links

Environment:

• Microsoft.Azure.ServiceBus 5.0.0
• .net sdk 3.1.102, Microsoft.NETCore.App 3.1.9
• Visual Studio 16.8.1
• Have verified the issue occurs on AzureServiceBus standard tier, I believe I have seen it on the premium tier as well.

---

Please let me know if you require any clarification from me.

Thank you for taking the time to look into this,
Paul Savides

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @axisc.

Issue Details

Description:	Describe the bug During periods when a large number of server side errors occur, we sometimes see messages getting "stuck" in queues. As in, they hang in the queues for the configured message lock timeout before being redelivered. After some attempts to reproduce with a smaller example, I have found that, in certain scenarios, when calling MessageReceiver.CloseAsync(), the inner ReceivingAmqpLink is not actually closed. So, the link is sitting there in the background, continually picking up messages The only way I was able to get this to reproduce is by closing & opening a new receiver when receiving an error on the ExceptionHandler. My best guess to why this issue occurs; when the inner link faults, it will auto-recover in OnReceiveAsync(). Perhaps there is a race condition with auto recovery & closing the receiver at similar times. Of course, perhaps there is something completely off with the usage of the sdk here as well. Expected behavior In general, I would expect that CloseAsync() would always close the inner ReceivingAmqpLink. To Reproduce Reproduction Repo = https://github.com/paulsavides/ServiceBusTesting ReproProject is the project that reproduces this issue. If the code is doing something extremely incorrect, please let me know. We are actually using the MassTransit library to interact with AzureServiceBus so I had to recreate a bit of what it was doing that reproduces the error. Open solution from production repo Set ReproProject as Startup project Fill in Endpoint & Shared Access Key Signature in Program.cs Run the project While the project is running, open the Queue in the Azure UI & continually update the Auto-delete after idle setting. This is an attempt to cause errors that requires the links to be recreated You can view this video to see exactly what I mean if the instructions are unclear https://www.youtube.com/watch?v=sv0bRozEevs Eventually, in the console output, you will see errors coming through the exception handler & the receiver will 'recycle' some number of times After recycle, you should start seeing message sends & receives being mismatched if not, go back to step 5 Press d to print out diagnostics on all of the links from "closed" receivers that are still open & the number of unsettled messages from those links Environment: Microsoft.Azure.ServiceBus 5.0.0 .net sdk 3.1.102, Microsoft.NETCore.App 3.1.9 Visual Studio 16.8.1 Have verified the issue occurs on AzureServiceBus standard tier, I believe I have seen it on the premium tier as well. Please let me know if you require any clarification from me. Thank you for taking the time to look into this, Paul Savides
Author:	paulsavides
Assignees:	-
Labels:	Client, Service Attention, Service Bus, customer-reported, needs-team-attention, needs-triage, question
Milestone:	-

[jsquire]

Thank you for your feedback. Tagging and routing to the team best able to assist.

[jsquire]

//fyi: @JoshLove-msft

[paulsavides]

I highly suspect this issue is due to the Microsoft.Azure.Amqp Singleton (and thus the FaultTolerantAmqpObject) class not properly handling concurrent closes and openings.

In my linked repo you can see the (failing) test here >> https://github.com/paulsavides/ServiceBusTesting/blob/4cf80ba2f8e2d725b2d1923600d4aba3f26ee9b3/InterestingTests/SingletonTests.cs#L12-L38

Although the singleton is closed & subsequent GetOrCreateAsync calls will fail, the internal value is not closed.

In the usage here, I believe the following two sections are being run in such an order to cause the issue I've replicated in that unit test.

During periods of transient errors, this section may enter the workflow to recreate the message receiver.

azure-sdk-for-net/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Core/MessageReceiver.cs

Lines 1078 to 1082 in d2bcf77

	if(!this.ReceiveLinkManager.TryGetOpenedObject(out receiveLink))
	{
	MessagingEventSource.Log.CreatingNewLink(this.ClientId, this.isSessionReceiver, this.SessionIdInternal, false, this.LinkException);
	receiveLink = await this.ReceiveLinkManager.GetOrCreateAsync(timeoutHelper.RemainingTime()).ConfigureAwait(false);
	}

If the receiver.CloseAsync() is called after entering the callback here >>

azure-sdk-for-net/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Core/MessageReceiver.cs

Line 1557 in d2bcf77

async Task<ReceivingAmqpLink> CreateLinkAsync(TimeSpan timeout)

It seems like it would be possible to end up with an orphaned receiver.

[JoshLove-msft]

/cc @xinchen10 to confirm the issue in the AMQP library.