search

FoundatioFx / Foundatio

Pluggable foundation blocks for building distributed apps.
Apache License 2.0
1.98k stars 244 forks source link

Race condition in Azure Service Bus queue #43

Open jamie94bc opened 8 years ago

jamie94bc commented 8 years ago

As mentioned by @moswald in #38 ...

  1. In parallel, Thread A calls EnqueueAsync() and Thread B calls DeleteQueueAsync()
  2. Thread A passes through EnsureQueueCreatedAsync(), ensuring _queueClient is not null
  3. Thread B deletes the queue, setting _queueClient to null
  4. Thread A tries to use _queueCleint which is now null

There's not really a nice way to handle this without locking. Couple of solutions:

Option 1

Wrap _queueClient in a property which throws a slightly different exception, leaving the user to retry.

Option 2

Implement retry logic (preferably in QueueBase).

niemyjski commented 8 years ago

Could use the new ? null check keyword which will help prevent against this too. I'm trying to think of a scenario when you would ever want to delete a queue..

jamie94bc commented 8 years ago

@niemyjski yeah I'm with you on that. I'd rather replace DeleteQueueAsync() with ClearQueueAsync()

jamie94bc commented 8 years ago

? would silently fail, which wouldn't be fantastic behavior!

niemyjski commented 8 years ago

Is this still an issue?

jamie94bc commented 8 years ago

Yeah - it's different from #45 (another race condition in AzureServiceBusQueue)

moswald commented 8 years ago

It is an issue, but I don't know if it can be solved without using locks everywhere. I think the "correct" solution is to document that it's a dangerous method and shouldn't be used in multi-threaded environments. Once one part of your application determines the queue should be deleted, the rest of the application should be halted.

niemyjski commented 8 years ago

@moswald would you mind updating the docs for this?