Skip to content

uasyncio submodule for synchronization primitives, Lock implemented.#224

Closed
pfalcon wants to merge 2 commits intomicropython:masterfrom
pfalcon:uasyncio-lock
Closed

uasyncio submodule for synchronization primitives, Lock implemented.#224
pfalcon wants to merge 2 commits intomicropython:masterfrom
pfalcon:uasyncio-lock

Conversation

@pfalcon
Copy link
Copy Markdown
Contributor

@pfalcon pfalcon commented Oct 29, 2017

@dpgeorge , @peterhinch : Please reviews. Points to consider:

  • Name of the submodule. I wished to use uasyncio.sync for qstr reuse, but that may be not clear enough.
  • Correctness of implementation. Note that this is compatible with CPython Lock API, which asyncio liking to confusingly introduce non-coroutines methods here and there, .release() being such. So, that was worked around by putting extra yield into acquire(), and even works without scheduling artifacts with the provided example, though in general, it might lead to such.

@peterhinch
Copy link
Copy Markdown
Contributor

Looks good to me.

@peterhinch
Copy link
Copy Markdown
Contributor

Why not support use in an asynchronous context manager as per this attempt?

@pfalcon
Copy link
Copy Markdown
Contributor Author

pfalcon commented Oct 30, 2017

Because context manager is a syntactic sugar requiring more code, and uasyncio's goal is to provide required functionality with the minimum of code.

@pfalcon
Copy link
Copy Markdown
Contributor Author

pfalcon commented Nov 2, 2017

Ok, I'm still not sure about "synchro" part, but there're more things in queue, so merging this. As I figure noone really tested this, I'm leaving debug statements in the code.

@pfalcon
Copy link
Copy Markdown
Contributor Author

pfalcon commented Nov 2, 2017

To clarify, this was developed to have an example on non-I/O-waiting coroutine which is fully suspended from the main scheduling queue (to be able to test cancellation algorithm and make sure that the proposed solution, which required patching the language core, is fully general).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants