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

Fix default scheduleOnce implementation #1057

Merged
merged 1 commit into from Oct 22, 2019

Conversation

@alexandru
Copy link
Member

alexandru commented Oct 22, 2019

Unfortunately we've got a nasty bug:

scheduler.scheduleOnce(0, MILLISECONDS, () => {
  println(Thread.currentThread().getName)
})

This will print monix-scheduler, because the logic will execute on Monix's thread-pool meant for scheduling stuff. And unfortunately this extends to task as well...

import scala.concurrent.duration._

Task.sleep(-1.millis) *> Task {
  println(Thread.currentThread().getName)
}

This too will print monix-scheduler.

@alexandru alexandru added the bug label Oct 22, 2019
Copy link
Collaborator

oleg-py left a comment

LGTM

scheduler: ScheduledExecutorService
)(
initialDelay: Long, unit: TimeUnit, r: Runnable
): Cancelable = {
if (initialDelay <= 0) {
executor.execute(r)

This comment has been minimized.

Copy link
@oleg-py

oleg-py Oct 22, 2019

Collaborator

I didn't realize initially that bug is on this line, because the actual diff looks like just rename

This comment has been minimized.

Copy link
@oleg-py

oleg-py Oct 22, 2019

Collaborator

Probably worth noting in commit message/release notes/etc. that it was triggered by using nonpositive delays

This comment has been minimized.

Copy link
@alexandru

alexandru Oct 22, 2019

Author Member

Indeed. I think it was the naming that made this confusing, hence the rename.

@Avasil
Avasil approved these changes Oct 22, 2019
@alexandru alexandru merged commit bfc2b51 into monix:master Oct 22, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@alexandru alexandru deleted the alexandru:fix-scheduleonce branch Oct 22, 2019
@alexandru

This comment has been minimized.

Copy link
Member Author

alexandru commented Oct 22, 2019

Released hash version: 3.0.0-bfc2b51

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.