python / asyncio

asyncio historical repository
https://docs.python.org/3/library/asyncio.html
1.04k stars 177 forks source link

Reentrant locks in asyncio #439

Closed ZhukovAlexander closed 7 years ago

ZhukovAlexander commented 7 years ago

Since asyncio.Lock is not reentrant, there is no way the same task could acquire the same lock more than once, so the following code will obviously block:

import asyncio

mutex = asyncio.Lock()

async def child_coro():
    async with mutex:
        print('Hello from chile_coro')

async def parent_coro():
    async with mutex:  # the code will block here
        print('Hello from parent_coro')
        await child_coro()

asyncio.get_event_loop().run_until_complete(parent_coro())

Is there any viable reason to implement an asyncio.RLock, similar to a threading.RLock?

gvanrossum commented 7 years ago

I doubt there will be much use for it. Even Lock isn't used that much, because the primary reason for using locks in a threaded world (prevent other threads from seeing inconsistent state while you are making a complex state update) is usually addressed quite simply by not using await while you are making the update. Since tasks are not reentrant, no other task will run while you are running unless you use await.

If you have a real use case for reentrant locks in asyncio we'll talk again.

ZhukovAlexander commented 7 years ago

@gvanrossum , fair enough. Closing the issue.

junqed commented 7 years ago

Here is the issue. We're writing a tool for our internal purpose. One of the functions is running git command. It uses asyncio.create_subprocess_exec to run them. Some of them are simple, some are composite (sets of simple operations). And consumers could call simple ones and composite alike.

Something like this:

class GitWrapper:
    git = None  # implementation of the git using create_subprocess_exec
    async pull(self):
        await self.git.pull()

   async checkout(self, branch='master')
       await self.git.checkout(branch)

    async new_branch(self, branch)
        await self.pull()
        await self.checkout()
        await self.git.new_branch(branch)

In new_branch another coroutine could call checkout between the calls and the whole result will be unpredictable. One thing I got in mind is just lock the whole GitWrapper, but possibly we could use inside of the GitWrapper? Or we do something wrong?

gjcarneiro commented 7 years ago

The way I would solve it is with a lock. But the point of this thread is that it doesn't need to be a recursive lock. I would split the pull and checkout methods into two versions, one taking the lock, one doesn't. new_branch can take the lock once, and then call the non-locking pull/checkout/new_branch.

class GitWrapper:
    git = None  # implementation of the git using create_subprocess_exec
    def __init__(self):
       self.lock = asyncio.Lock()

    async def _pull(self):
        await self.git.pull()

    async def pull(self):
          async with self.lock:
                await self._pull()

    async def _checkout(self, branch='master'):
       await self.git.checkout(branch)

    async def checkout(self, branch='master'):
       async with self.lock:
            await self._checkout(branch)

    async def new_branch(self, branch):
        async with self.lock:
            await self._pull()
            await self._checkout()
            await self.git.new_branch(branch)
junqed commented 7 years ago

@gjcarneiro you mean:

async def new_branch(self, branch)
        async with self.lock:
            await self._pull()
            await self._checkout()
            await self.git.new_branch(branch)

?

gjcarneiro commented 7 years ago

yes, @junqed, that's what I meant, sorry. Also missing : at the end of def async def line :P Will edit.