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

ExplicitlyTriggeredScheduler does not cancel tasks in Akka Testkit #28604

Closed
bszwej opened this issue Feb 12, 2020 · 3 comments
Closed

ExplicitlyTriggeredScheduler does not cancel tasks in Akka Testkit #28604

bszwej opened this issue Feb 12, 2020 · 3 comments
Milestone

Comments

@bszwej
Copy link
Contributor

@bszwej bszwej commented Feb 12, 2020

Hi,

In Akka Testkit, there's ExplicitlyTriggeredScheduler, that allows to test scheduling logic by moving in time.

The problem is, that when a task is scheduled, then it's impossible to cancel it. See the following snippet:

object MinimalExample extends App {
  val scheduler = new ExplicitlyTriggeredScheduler(null, printlnAdapter, null)
  val task = scheduler.schedule(1.seconds, 1.second)(println("Hello world!"))(Implicits.global)
  
  scheduler.timePasses(10 seconds)
  println(task.cancel())
  scheduler.timePasses(10 seconds)

  // doesn't matter...
  lazy val printlnAdapter = new LoggingAdapter {
    override def isErrorEnabled: Boolean = false
    override def isWarningEnabled: Boolean = false
    override def isInfoEnabled: Boolean = false
    override def isDebugEnabled: Boolean = true
    override protected def notifyError(message: String): Unit = println(message)
    override protected def notifyError(cause: Throwable, message: String): Unit = println(message)
    override protected def notifyWarning(message: String): Unit = println(message)
    override protected def notifyInfo(message: String): Unit = println(message)
    override protected def notifyDebug(message: String): Unit = println(message)
  }
}

Output:

Scheduled item for 1000: Item(1000,Some(1 second),akka.actor.Scheduler$$anon$2@1c72da34)
Time proceeds from 0 to 10000, currently scheduled for this period:List(
- Item(1000,Some(1 second),akka.actor.Scheduler$$anon$2@1c72da34))
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
false // println(task.cancel())
Time proceeds from 10000 to 20000, currently scheduled for this period:List(
- Item(11000,Some(1 second),akka.actor.Scheduler$$anon$2@1c72da34))
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!

As you can see task.cancel() returns false, meaning, that canceling was unsuccessful. Job execution does not stop.

Expected output would be:

Scheduled item for 1000: Item(Some(1 second),akka.actor.Scheduler$$anon$2@6b67034)
Time proceeds from 0 to 10000, currently scheduled for this period:List(
- (Item(Some(1 second),akka.actor.Scheduler$$anon$2@6b67034),1000))
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
Hello world!
true
Time proceeds from 10000 to 20000, currently scheduled for this period:List()

I'm using Akka 2.5.27, but the same behavior can be observed on 2.5.29 and 2.6.3.

Let me know what do you think. I'd be more than happy to create a PR with the fix.

@bszwej bszwej changed the title ExplicitlyTriggeredScheduler does not cancel tasks [Akka Testkit] ExplicitlyTriggeredScheduler does not cancel tasks in Akka Testkit Feb 12, 2020
@raboof

This comment has been minimized.

Copy link
Member

@raboof raboof commented Feb 13, 2020

That's interesting - there does seem to be some logic there to remove it from the scheduled items, so not sure what's going wrong there. It'd be great if you could look into that!

@slouc

This comment has been minimized.

Copy link

@slouc slouc commented Feb 14, 2020

That's interesting - there does seem to be some logic there to remove it from the scheduled items, so not sure what's going wrong there. It'd be great if you could look into that!

Hi @raboof , let me try to answer this one.

Whenever we want to pass some time, executeTasks gets invoked. It goes through all scheduled tasks in the scheduled map and, for each task:

  1. Removes the task from the map,
  2. Inserts a copy of the task with incremented time to the map.

Now let's say we schedule a task T, then pass some time, and then we want to cancel it. Invoking cancel() on T attempts to remove it from scheduled map. But T is no longer in the map; when the time was passed, T was removed, and its copy with incremented time (let's call it T') has been inserted instead.

So now we are forever stuck with a stale reference to an already-cancelled task T, unable to cancel its successor T'. If we again pass some time, T' will be removed, but T'' will be scheduled, and so on. This effectively makes it seem in our test as if our original task T keeps running on every time tick, even though we wanted to cancel it.

@bszwej

This comment has been minimized.

Copy link
Contributor Author

@bszwej bszwej commented Feb 14, 2020

Hi @raboof
The fix is ready: #28614

johanandren pushed a commit that referenced this issue Mar 11, 2020
@johanandren johanandren added this to the 2.6.4 milestone Mar 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.