Skip to content

Conversation

@helena
Copy link

@helena helena commented Nov 17, 2010

test of whitespace commit from fork

@dsyer
Copy link
Member

dsyer commented Nov 17, 2010

Another whitespace change. Not bothering to merge this one.

garyrussell added a commit to garyrussell/spring-amqp that referenced this pull request Jul 14, 2016
1. Fix case when basicCancel received with only one queue (set cancelled).
2. Add protection to cancelled() in case all cancelOks are not received within shutdownTimeout
   after aborting.
3. If we are aborting, use offer instead of put to insert into the queue to avoid indefinitely
   blocking the client thread.
4. Purge the queue at the end of stop() just in case spring-projects#2 happens and we still have a hung client thread.
garyrussell added a commit to garyrussell/spring-amqp that referenced this pull request Jul 14, 2016
1. Fix case when basicCancel received with only one queue (set cancelled).
2. Add protection to cancelled() in case all cancelOks are not received within shutdownTimeout
   after aborting.
3. If we are aborting, use offer instead of put to insert into the queue to avoid indefinitely
   blocking the client thread.
4. Purge the queue at the end of stop() just in case spring-projects#2 happens and we still have a hung client thread.
garyrussell pushed a commit that referenced this pull request Jul 15, 2016
JIRA: https://jira.spring.io/browse/AMQP-621

The `SimpleMessageListenerContainer` has logic to initiate `BlockingQueueConsumer.basicCancel()` and mark `BlockingQueueConsumer` as inactive immediately.
But the `handleCancelOk` answer can come back from the Broker a bit late, in async manner.
During this time window we can still receive messages via `handleDelivery`.
With `prefetchCount = 1` we may end up with the deadlock on `handleDelivery` thread during that time window, when main consumer loop identifies consumer as inactive and exits for `stop` process
and can't do that because the `BlockingQueueConsumer.this.queue.put(new Delivery(consumerTag, envelope, properties, body))` operation blocks the consumer from any other operations.

* Fix the race condition via additional `BlockingQueueConsumer.cancelled()` state to let main loop to spin until `handleCancelOk` honoring any in-flight messages
* The `cancelled` state is based on the `cancelReceived` flag which is changed from the `handleCancelOk` as a Broker response for the `basicCancel`
and in the `handleCancel` event from the Broker as a reason of some unexpected state on Broker, e.g. consuming queue has been removed
* Protect message lost via `basicReject` in case of late arrival into `handleDelivery`, when the consumer has been cancelled already

Address PR comments

* Rename `cancelReceived` -> `cancelled` with JavaDocs
* Initiate `basicCancel()` from `handleCancel(String consumerTag)` to cancel `Consumer` from all other tags.
The Broker canceling initiative is fully similar to `queueChanged` in the listener container
* Do no mark `cancelled` in the `handleCancelOk` until all the tags are cancelled

Polishing

1. Fix case when basicCancel received with only one queue (set cancelled).
2. Add protection to cancelled() in case all cancelOks are not received within shutdownTimeout
   after aborting.
3. If we are aborting, use offer instead of put to insert into the queue to avoid indefinitely
   blocking the client thread.
4. Purge the queue at the end of stop() just in case #2 happens and we still have a hung client thread.

Add Test - Abort with Multiple Queues
artembilan pushed a commit that referenced this pull request Aug 15, 2024
Bumps [io.micrometer:micrometer-bom](https://github.com/micrometer-metrics/micrometer-commercial) from 1.10.13 to 1.10.15.
- [Commits](https://github.com/micrometer-metrics/micrometer-commercial/commits)

---
updated-dependencies:
- dependency-name: io.micrometer:micrometer-bom
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants