Skip to content
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.

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

2.x: schedulePeriodically with non-positive period #5416

Closed
akarnokd opened this issue Jun 14, 2017 · 3 comments
Closed

2.x: schedulePeriodically with non-positive period #5416

akarnokd opened this issue Jun 14, 2017 · 3 comments

Comments

@akarnokd
Copy link
Member

Currently, the Scheduler and Worker API allows for non-positive periods on periodically scheduling but the underlying ScheduledExecutorServices don't. The javadoc doesn't specify any restrictions on period unlike initialDelay or delay where a non-positive value is considered to be an "execute without delay".

In theory, the default periodic logic could handle the case by executing such periodic tasks without any delay but I'm not certain the behavior is actually desired.

Related StackOverflow question: https://stackoverflow.com/questions/44555253/spring-boot-and-rxjava2-integration-nullpointerexception-actually-not

So the tw options I see are as follows:

  1. throw IAE with non-positive periods with the related operators and require the Scheduler/Worker API to do the same
  2. allow 0 period and introduce workarounds inside the Scheduler/Worker implementations.

I prefer option 1 as nobody seem to have encountered this specific issue before that would have rendered the code inoperable anyway due to a crash similar to the SO case.

@Kiskae
Copy link
Contributor

Kiskae commented Jun 16, 2017

It looks like the Scheduler.Worker.schedulePeriodically javadoc does specify the behavior for non-positive values:

     * Note to implementors: non-positive {@code initialTime} and {@code period} should be regarded as
     * non-delayed scheduling of the first and any subsequent executions.
     * @param period
     *            the time interval to wait each time in between executing the action; non-positive values
     *            indicate no delay between repeated schedules

So in the case of the linked stackoverflow question its NewThreadWorker not following the contract specified by the javadoc. In addition there is the question whether the contract for Scheduler.schedulePeriodicallyDirect should be the same as Schduler.Worker.schedulePeriodically

The question then becomes either changing the contract for option 1 or more extensive testing of existing schedulers for option 2.

@akarnokd
Copy link
Member Author

Thanks for your findings. If it says in the javadoc then the schedulers should honor it. I'll prepare a PR that fixes and checks for such behavior.

@akarnokd
Copy link
Member Author

Closing via #5419.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants