Skip to content

Conversation

@clue
Copy link
Member

@clue clue commented Mar 26, 2018

This PR fixes a subtle bug that can be triggered by cancelling a queued middleware request handler, i.e. before it is actually started (pending). If too many handlers are already pending, cancelling one of the queued handlers should not invoke another handler. On the other hand, cancelling a pending handler should invoke the next queued handler.

This is a subtle situation that is explicitly covered by the new tests added (see last test case). The remaining test cases exist to ensure we do not introduce any regressions and this stays at 100% coverage.

I noticed this while implementing https://github.com/clue/php-mq-react and basically backported the same logic to this project 👍

@clue clue added the bug label Mar 26, 2018
@clue clue added this to the v0.8.2 milestone Mar 26, 2018
@WyriHaximus WyriHaximus requested review from WyriHaximus and jsor March 26, 2018 21:35
@jsor jsor merged commit 327bdfe into reactphp:master Mar 27, 2018
@clue clue deleted the queued-cancellation branch March 27, 2018 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants