Skip to content

Conversation

ysbaddaden
Copy link
Contributor

@ysbaddaden ysbaddaden commented Aug 7, 2025

Initial attempt to implement RFC 0014 that aims to introduce a cancelable timer, designed to be low level, yet simple and efficient, allowing higher level abstractions, such as a pool of connections for example, to easily add timeout features.

I found an implementation gotcha (no impact on the RFC): we MUST synchronize when a fiber canceled the timeout while the event loop is running and has already dequeued the timer. Because the event/timer objects are allocated on the stack, so we must suspend again until the evloop has processed the timer (otherwise 💥) and the evloop must always enqueue. We could allocate on the HEAP as I had to do for libevent (no way to know if the event was deleted), but we'd have to always allocate because there wouldn't be any way to know if the event changed (and could cancel a new timeout using the new token 💣).

In practice the extra Fiber.suspend should be rare, and I feel better knowing that there are no side effects after the timeout returns.

I checked the internal usages for timeouts in the IOCP and Epoll/Kqueue event loops, and I'm pretty confident they could use (at least Fiber#new_cancelation_token, Fiber#resolve_timer? with their internal #sleep implementation, to avoid a Fiber -> EventLoop -> Fiber -> EventLoop dependency). I didn't check the usage in the select timeout action, yet, but I hope we can reuse the cancelation mechanism instead of the select state allocation (regardless of the presence of a timeout action).

NOTE: this is untested, not even compilation. CI will likely fail all over 🙈 Now fixed and verified on Linux (polling & libevent) and Windows (IOCP).

@ysbaddaden ysbaddaden self-assigned this Aug 7, 2025
@ysbaddaden ysbaddaden force-pushed the feature/rfc-14-cancelable-timers branch 2 times, most recently from cdd9a84 to c6ed7c7 Compare August 8, 2025 16:09
@ysbaddaden ysbaddaden force-pushed the feature/rfc-14-cancelable-timers branch from c6ed7c7 to 72c66d8 Compare August 8, 2025 16:12
@ysbaddaden ysbaddaden marked this pull request as ready for review August 8, 2025 16:31
@straight-shoota
Copy link
Member

Any chance to get some specs for this?

@ysbaddaden
Copy link
Contributor Author

Sure, here they come!

- `Fiber::CancelationToken` instead of `TimeoutToken` so we can
  internally reuse the mechanism for select regardless of the presence
  of a timeout action

- `Fiber.sleep(Time::Span, & : CancelationToken -> TimeoutResult)`
  instead of `.timeout`

- `Fiber#resolve_timer?(token : CancelationToken) : Bool` instead of
  `#resolve_timeout?`

- `::sleep(Time::Span)` becomes a shortcut for `Fiber.sleep(Time::Span) { }`

- `Crystal::EventLoop#sleep(Time::Span, CancelationToken)` to replace
  the existing method instead of introducing a new `#timeout` method
@ysbaddaden ysbaddaden changed the title RFC 0014: Add Fiber.timeout RFC 0014: Cancelable sleep Aug 26, 2025
@crysbot
Copy link
Collaborator

crysbot commented Aug 28, 2025

This pull request has been mentioned on Crystal Forum. There might be relevant details there:

https://forum.crystal-lang.org/t/rfc-14-cancelable-timers/8386/1

@ysbaddaden
Copy link
Contributor Author

Back to draft. There are things to resolve around "cancellation" that is more general than "canceling a timer".

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

Successfully merging this pull request may close these issues.

3 participants