6.x: Guard against null when removing ShutdownHandler

[Link9001]

Proposed Changes

Solving Bug #1858
A race condition occurs when a RabbitMQ client and server simultaneously try to cancel the same consumer. This can happen, for example, if the queue being consumed is deleted on the server at the same moment the client cancels the consumer.

This creates a conflict in ModelBase.cs between the HandleBasicCancel and HandleBasicCancelOk methods. Both methods attempt to remove the consumer, but they operate differently:

• HandleBasicCancel is initiated by the server and directly removes the consumer.
• HandleBasicCancelOk is a confirmation from the server for a client-initiated cancellation. It tries to schedule a final action using the consumer object.

If HandleBasicCancel executes first, it removes the consumer. When HandleBasicCancelOk subsequently runs, it attempts to operate on a consumer that no longer exists, leading to an error when BasicCancel tries to remove the Shutdown EventHandler.

The fix for this race condition is relatively straightforward. The solution is to process the cancellation requests on a FCFS basis while adding null checks to ensure that all subsequent methods to execute does not cause an error by trying to access a consumer that has already been removed. (To my Knowlage it is only HandleBasicCancel and HandleBasicCancelOk)

Types of Changes

What types of changes does your code introduce to this project?
Put an x in the boxes that apply

• Bug fix (non-breaking change which fixes issue A NullReferenceException occurs when canceling a consumer (Library Version 6.8.1) #1858)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause an observable behavior change in existing systems)
• Documentation improvements (corrections, new content, etc)
• Cosmetic change (whitespace, formatting, etc)

Checklist

• I have read the CONTRIBUTING.md document
• [?] I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq) - I can't get to this link. Is it valid?
• All tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• [-] I have added necessary documentation (if appropriate)
• [-] Any dependent changes have been merged and published in related repositories

Further Comments

[Link9001]

Version 6.x still uses outdated nuget packages with security vulnerabilities (e.g. System.Text.Json 6.0, …) and will not let it build properly. Should I include the nuget packages update also in this pull request, or are you handling that differently?

[michaelklishin]

Multiple other clients do exactly this. Perhaps with a warning emitted but I don't feel strongly about that since the intent of such scenario is to cancel a consumer, and with manual acknowledgements having, say, a default "catch-all" consumer (that the Java client supports) is not really necessary in most cases.

[lukebakken]

Should I include the nuget packages update also in this pull request, or are you handling that differently?

@Link9001 would you mind opening a new PR based on the 6.x branch to update those deps to the minimum safe version? Thank you.

[lukebakken]

Thank you!