dabeaz / curio

Good Curio!
Other
4.01k stars 240 forks source link

Asyncio breaking between 1.1 and 1.2 #330

Closed borisuu closed 3 years ago

borisuu commented 3 years ago

System: Python 3.8 curio 1.2 Windows 64-bit asyncio bridge (copied from the 0.9 version)

I'm having a problem when upgrading from 1.1 to curio 1.2. I'm working with a fairly complex system, 99% of which runs on pure curio, but I still have a dependency to aio_pika which has no curio client (yet). The code runs normally on 1.1, but breaks on 1.2. I saw that the contextvars were removed between the two versions and used the ContextTask in the kernel instead. Sadly that didn't help as the process just hangs at some point. I suppose it's a problem with one of the futures not resolving its result, or a callback not being executed properly. Any ideas what could be causing the issue? Any help is welcome.

EDIT: I'm using an UniversalQueue to put messages through aio_pika's callback into my system. It seems that's where it all breaks down as the await queue.get() never gets re-scheduled (might be wrong tho).

dabeaz commented 3 years ago

Hmmmm. Just confirming that it works on curio 1.1 and not on curio 1.2. I'm looking at the CHANGES file and it doesn't appear that a whole lot changed between versions. However, focusing on what did change might be a starting point.

borisuu commented 3 years ago

I found the problem, sadly forgot to mention that the tests are breaking. It was just the fact that the 1.2 package introduces the curio mark and kernel fixture, which I already had in my codebase. Then the kernel would start all fixture tasks on my kernel and the rest on the curio kernel fixture.

We should think of a way to avoid that in the future and also make the kernel fixture configurable, in order to use the ContextTask in the kernel.

borisuu commented 3 years ago

Now that I've inspected the pytest_plugin, two things come to mind:

  1. The plugin should be made optionally installable
  2. The curio mark runs the test with an anonymous kernel and leads to a problem when using the kernel fixture

@dabeaz I know you're not looking for maintainers or anything of the sort, but I would be prepared to make a PR if that is ok with you.

dabeaz commented 3 years ago

This is the second reported breakage on the pytest_plugin. I'm now thinking about removing it from the Curio installation or making it part of the examples directory. Open to thoughts.

borisuu commented 3 years ago

Either the examples directory or make a separate package and reference it in the extras_require. This way anyone who wants it can install it. I think both options are valid.

If you go the separate package route, I'd make a second key, e.g. 'pytest_runner': ['curio_pytest'], just to make it extra clear it's optional even in the context of testing.

borisuu commented 3 years ago

I'm closing this as it seems you'll be moving the pytest_plugin to the examples. Thumbsup to that.