cloudflare / cloudflare/workerd

`setTimeout` and `fetch` never callback/fulfill when DO gets evicted

Open
#1,185 8 comments 1 reaction 0 assignees View on GitHub
Durable Objects
Dominant language
C++
Stars
8.7k
Forks
739
Avg merge
2d 20h
Merged PRs (30d)
174

Description

`fetch` returns a promise that should eventually be fulfilled (by either resolve or reject). When a DO is evicted, this promise never fulfills.

Often this is fine because the execution environment is destroyed, so the fact that the fetch never fulfills is not observable. But in the case where the execution environment is reused, ie because a new DO is instantiated within it, application code can observe that `fetch` never fulfills.

A similar thing happens with `setTimeout` and `setInterval`. If you set a timer from a DO and the DO is evicted, the callback for the timer is not called. Normally this is fine because the execution environment goes away. But in the case where execution environment is reused, it can lead to weird results.

One example we have run into where this hurt us:

- We have a log sink in a DO that we want to flush (by sending logs to a server) periodically
- We set a periodic timer to send the logs
- Somebody accidentally makes this sink global
- When a DO is unloaded, the timer stops getting called
- We stop sending logs
- Eventually the log buffer fills up and the program crashes
- We don't have any logs prior to the crash because they weren't getting sent

Here is a GitHub repo demonstrating the above issue:

https://github.com/rocicorp/cf-global-state/blob/main/README.md

We are aware of the advise to not use global variables and do attempt to follow it. However they have crept into our worker code a number of times by accident, frequently with this kind of pretty serious problem. It is difficult to completely avoid globals in larger code bases, since they are part of the javascript ecosystem.

I think that if JS contexts are going to get reused, then the platform should ensure any waiting promises and callbacks do get fired. If the promises are going to be abandoned, then the js context should also be killed so that it's not detectable. Otherwise the behavior seems inconsistent with the contract of these APIs.

Another solution that would work for us would be to provide an option to always load DOs into a new `v8::Context`. This would slightly slow down the launch of DOs, but would completely avoid the problem with global variables since global code would re-run at the start of each DO instantiation.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.