Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Temporary workaround to avoid dropping a tokio::runtime while another is running #1991

Merged
merged 1 commit into from
Jul 9, 2021

Conversation

jyn514
Copy link
Contributor

@jyn514 jyn514 commented Jul 9, 2021

This fixes the following error from wrangler tail:

$ wrangler tail
🦚  Setting up log streaming from Worker script "test-project". Using ports 8080 and 8081.
This may take a few seconds...

Oops! wrangler encountered an error... please help Cloudflare debug this issue by submitting an error report (/home/jnelson/.wrangler/errors/1625847335787.log)

To submit this error report to Cloudflare, run:

    $ wrangler report

Closing tail session...
Error: panic

This is a regression from #1966, since wrangler now makes API calls in more places than it used to. The panic in that file is "Cannot drop a runtime in a context where blocking is not allowed. This happens when a runtime is dropped from within an asynchronous context."
The larger issue is that there are many different runtimes in Wrangler:

  • A runtime created by wrangler tail to watch for changes
  • A runtime created by reqwest::blocking to fetch the account ID from Cloudflare's API
  • A runtime in commands::dev::gcs - not sure what this does, something to do with a persistent server?
  • A runtime in commands::dev::edge - ditto

Having more than one runtime going at once is not great; the three in Wrangler
seem to be disjoint, but the one in cloudflare_rs absolutely overlaps with some
of the ones from wrangler.

This is a temporary workaround to avoid panics in wrangler tail, but I'm
working on a larger fix to cloudflare_rs to see if it can avoid spawning a new
runtime for each request.

cc @jspspike @nilslice @Electroid

@jyn514 jyn514 added the bug Something isn't working label Jul 9, 2021
@jyn514 jyn514 requested a review from a team as a code owner July 9, 2021 16:24
Copy link
Contributor

@jspspike jspspike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and tail seems to be working properly

Copy link
Contributor

@nilslice nilslice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Also tested and verified the fix - works as expected

… is running

This fixes the following error from `wrangler tail`:

```
$ wrangler tail
🦚  Setting up log streaming from Worker script "test-project". Using ports 8080 and 8081.
This may take a few seconds...

Oops! wrangler encountered an error... please help Cloudflare debug this issue by submitting an error report (/home/jnelson/.wrangler/errors/1625847335787.log)

To submit this error report to Cloudflare, run:

    $ wrangler report

Closing tail session...
Error: panic
```

The panic in that file is "Cannot drop a runtime in a context where blocking is not allowed. This happens when a runtime is dropped from within an asynchronous context."
The larger issue is that there are many different runtimes in Wrangler:
- A runtime created by `wrangler tail` to watch for changes
- A runtime created by `reqwest::blocking` to fetch the account ID from Cloudflare's API
- A runtime in `commands::dev::gcs` - not sure what this does, something to do with a persistent server?
- A runtime in `commands::dev::edge` - ditto

Having more than one runtime going at once is not great; the three in Wrangler
seem to be disjoint, but the one in cloudflare_rs absolutely overlaps with some
of the ones from wrangler.

This is a temporary workaround to avoid panics in `wrangler tail`, but I'm
working on a larger fix to cloudflare_rs to see if it can avoid spawning a new
runtime for each request.
@jyn514 jyn514 merged commit 67821fc into master Jul 9, 2021
@delete-merged-branch delete-merged-branch bot deleted the jnelson/async-tail-fix branch July 9, 2021 20:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants