Timed infinite loop fix

[strohel]

timed tests: rename test, assert only after shutdown, extend timeline comments

Add test for issue #79

Not currently failing, just demonstrates the unfortunate behavior.

timed: fix #79 by enqueueing (rather than handling) delayed messages when due

Also rename fire_at to enqueue_at to be explicit about the fact.

This is a trade-off that prevents 2 sorts of bad behavior:

• actors with self-sending messages never handling any delayed/recurring messages (#72)
• actors with recurring messages that are slower to handle than their interval eventually not processing any outside messages (#79)

See the tweaked tests for the change of behavior.

---

Checking how the test results change commit-by-commit should give a good story.

[strohel]

@bschwind @goodhoko I've pushed a fixup commit that extends comments as suggested (+ also extends the docs directly on TimedMessage variants). PTAL.

[strohel]

I suppose this can be released with a mere patch bump. Or do we consider the change in delivery order to be breaking?

Good point. The rename of the TimedMessage::Delayed fire_at field to enqueue_at is technically semver-incompatible, so we should bump the minor version. It will at make it easier for donwstreams to downgrate actor version shuold they need it.

I'll prepare portal version that uses this PR before merging to get some real-life testing. It doesn't technically require any code changes (the renamed fields are not directly used). Also opportunity for @bschwind to check clarity of comments of the latest version.

[goodhoko]

The rename of the TimedMessage::Delayed fire_at field to enqueue_at is technically semver-incompatible

Ahaaa! I assumed that the enum field is private by default (as are struct fields) but actually enum variants and their fields always share the visibility of the enum itself. I.e. the field is part of the crate's public API.

so we should bump the minor version. It will at make it easier for donwstreams to downgrate actor version shuold they need it.

Per the semver spec, if it's backward incompatible we should bump the major version. Minor version bumps are for additions (backward compatible changes). It feels excessive but hey, it's just a version number.

[strohel]

Per the semver spec, if it's backward incompatible we should bump the major version. Minor version bumps are for additions (backward compatible changes). It feels excessive but hey, it's just a version number.

Yup, but 0.x.y versions are special, in both semver and cargo's semantics https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-cratesio

• 0.x.y means "pre-production" in both. actor is arguably production-ready now, though I don't feel like doing that step (of updating from 0.x to 1.0) now
• SemVer says e.g. 0.3.0 and 0.3.1 are never compatible
• cargo says that 0.3.1 can be used when "0.3.0" or "0.3" are specified (patch version backward compatibility)
  • 0.3 and 0.4 are always incompatible even with cargo (while 1.4 can be used when 1.3 is requsted)

[goodhoko]

@strohel Got it. I didn't realize actor is still pre-1.0. Thanks!

[strohel]

Tonari internal note: if we decide to revive this, the logic in https://github.com/tonarino/portal/pull/3692 may stop working.