Skip to content

fix: bound and short-circuit the ToxicDelete deadlock on dead sinks - #760

Open
matheusgb wants to merge 1 commit into
Shopify:mainfrom
matheusgb:fix/toxic-delete-deadlock
Open

fix: bound and short-circuit the ToxicDelete deadlock on dead sinks#760
matheusgb wants to merge 1 commit into
Shopify:mainfrom
matheusgb:fix/toxic-delete-deadlock

Conversation

@matheusgb

Copy link
Copy Markdown

Toxics (noop, latency) forwarded chunks with a plain, unbounded stub.Output <- chunk. If nothing was left reading that channel, the send blocked forever and could never be interrupted, so RemoveToxic hung while holding the collection's mutex, freezing the whole control API (reads included).

  • ToxicStub.OutputDone and ToxicLink.writeDone let a send react instantly when the next toxic already closed itself, or the real destination connection already died.
  • Otherwise, the send is bounded by a timeout derived from the chain's own configured delay, so a legitimately slow chain isn't mistaken for a dead one.
  • Draining a removed toxic's leftover queue no longer retries a known-dead sink item by item.

Fixes #558, #427, #428.

Toxics (noop, latency) forwarded chunks with a plain, unbounded
`stub.Output <- chunk`. If nothing was left reading that channel, the
send blocked forever and could never be interrupted, so RemoveToxic
hung while holding the collection's mutex, freezing the whole control
API (reads included).

- ToxicStub.OutputDone and ToxicLink.writeDone let a send react
  instantly when the next toxic already closed itself, or the real
  destination connection already died.
- Otherwise, the send is bounded by a timeout derived from the chain's
  own configured delay, so a legitimately slow chain isn't mistaken
  for a dead one.
- Draining a removed toxic's leftover queue no longer retries a
  known-dead sink item by item.

Fixes Shopify#558, Shopify#427, Shopify#428.
@matheusgb

Copy link
Copy Markdown
Author

I have signed the CLA!

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.

Toxiproxy control port hangs on removing a toxic

1 participant