10000 [Messenger] MultiplierRetryStrategy int type overflow · Issue #57779 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Messenger] MultiplierRetryStrategy int type overflow #57779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
rela589n opened this issue Jul 19, 2024 · 0 comments
Closed

[Messenger] MultiplierRetryStrategy int type overflow #57779

rela589n opened this issue Jul 19, 2024 · 0 comments
Labels
Bug Messenger Status: Needs Review

Comments

@rela589n
Copy link
Contributor

Symfony version(s) affected

7.1

Description

Currently we face following error:

{
    "class": "ValueError",
    "message": "random_int(): Argument #1 ($min) must be less than or equal to argument #2 ($max)",
    "file": "vendor/symfony/messenger/Retry/MultiplierRetryStrategy.php:87",
    "trace": [
        "vendor/symfony/messenger/Retry/MultiplierRetryStrategy.php:87",
        "vendor/symfony/messenger/EventListener/SendFailedMessageForRetryListener.php:66",
        "vendor/symfony/event-dispatcher/EventDispatcher.php:246",
        "vendor/symfony/event-dispatcher/EventDispatcher.php:206",
        "vendor/symfony/event-dispatcher/EventDispatcher.php:56",
        "vendor/symfony/messenger/Worker.php:198",
        "vendor/symfony/messenger/Worker.php:174",
        "vendor/symfony/messenger/Worker.php:109",
        "vendor/symfony/messenger/Command/ConsumeMessagesCommand.php:244",
        "vendor/symfony/console/Command/Command.php:279",
        "vendor/symfony/console/Application.php:1047",
        "vendor/symfony/framework-bundle/Console/Application.php:123",
        "vendor/symfony/console/Application.php:316",
        "vendor/symfony/framework-bundle/Console/Application.php:77",
        "vendor/symfony/console/Application.php:167",
        "vendor/symfony/runtime/Runner/Symfony/ConsoleApplicationRunner.php:49",
        "vendor/autoload_runtime.php:29",
        "bin/console:15"
    ]
}

It seems that $randomness is overflowed in such way that it becomes a negative integer.

Hence, random_int stumbles over it:

$randomness = (int) ($delay * $this->jitter);
$delay += random_int(-$randomness, +$randomness);

How to reproduce

  1. Configure async transport with quite high max retries count:
framework:
    messenger:
        transports:
            async_two_phase_commit:
                dsn: '%env(MESSENGER_TRANSPORT_DSN)%'
                options:
                    auto_setup: true
                    exchange:
                        name: two_phase_commit
                    queues:
                        messages_two_phase_commit: ~
                retry_strategy:
                    max_retries: 512
                    delay: 1000
                    multiplier: 2
                    max_delay: 7_200_000 # 2 hours
  1. Configure message routing to this transport:
framework:
    messenger:
        routing:
            App\YourMessage: async_two_phase_commit
  1. Dispatch your deliberately failing message into the bus:
    public function __construct(
        private MessageBusInterface $bus,
    ) {
    }

    public function __invoke(): void
    {
        $this->bus->dispatch(new YourMessage());
    }
  1. Wait until delay overflows
  2. See that after quite a time the error bloats up in a tremendous number of failures in the logs

Possible Solution

In case when $this->maxDelayMilliseconds is configured (2 hours in the above example), it would make sense not to calculate the hypothetical delay that could've been in case if that option was absent.

Additional Context

No response
@rela589n rela589n added the Bug label Jul 19, 2024
@rela589n rela589n changed the title MultiplierRetryStrategy int type overflow [Messenger] MultiplierRetryStrategy int type overflow Jul 19, 2024
@alexandre-daubois alexandre-daubois mentioned this issue Aug 6, 2024
Merged
nicolas-grekas added a commit that referenced this issue Aug 8, 2024
@nicolas-grekas
bug #57933 [Messenger] Prevent waiting time to overflow when using lo…
6e7fe3e 
…ng delays (alexandre-daubois)

This PR was merged into the 7.1 branch.

Discussion
----------

[Messenger] Prevent waiting time to overflow when using long delays

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #57779
| License       | MIT

There are two places where integers can overflow, thus the limiting to `PHP_INT_MAX`.

Commits
-------

9609e23 [Messenger] Prevent waiting time to overflow when using long delays
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Messenger Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@nicolas-grekas @xabbuh @carsonbot @rela589n
0