Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CancelScope.cancel() does not work as expected if called within move_on_after() context #370

Closed
arthurxlee opened this issue Sep 12, 2021 · 7 comments · Fixed by #371
Closed
Labels
asyncio Involves the asyncio backend bug Something isn't working

Comments

@arthurxlee
Copy link

I ran this:

from anyio import move_on_after, sleep, run, CancelScope

result1 = 0
result2 = 0

async def main1():
    global result1
    with CancelScope() as ascope:
        with move_on_after(1, shield=True) as scope:  # XXX
            with CancelScope(shield=True):
                print("Starting sleep")
                ascope.cancel()
                print("ascope cancelled")
                await sleep(2)

                print( "Cancel scopes=", scope.cancel_called, ascope.cancel_called)

        print("zzzz")
        await sleep(0)

        result1 = 1
        print("Exited cancel scopes=", scope.cancel_called, ascope.cancel_called)

async def main2():
    global result2
    with CancelScope() as ascope:
        with move_on_after(1) as scope:  # XXX
            with CancelScope(shield=True):  # XXX
                with CancelScope(shield=True):
                    print("Starting sleep")
                    ascope.cancel()
                    print("ascope cancelled")
                    await sleep(2)

                    print("Cancel scopes=", scope.cancel_called, ascope.cancel_called)

        print("zzzz")
        await sleep(0)

        result2 = 1
        print("Exited cancel scopes=", scope.cancel_called, ascope.cancel_called)

run(main1); print()
run(main2); print()
assert result1 == result2, "main1() and main2() should be equivalent"

And the outcome is:

Starting sleep
ascope cancelled
Cancel scopes= True True
zzzz

Starting sleep
ascope cancelled
Cancel scopes= True True
zzzz
Exited cancel scopes= True True

Traceback (most recent call last):
  File "C:\tmp\whichmove.py", line 45, in <module>
    assert result1 == result2, "main1() and main2() should be equivalent"
AssertionError: main1() and main2() should be equivalent

This came from win10, python 3.9.6, asyncio, anyio-3.3.1

From my reading of the documentation both main1 and main2 should be equivalent. If that is the case, I would say there is an issue with main2. Thank you.

@arthurxlee
Copy link
Author

I have some time in my hand and zoomed in further into the issue

from anyio import move_on_after, sleep, run, CancelScope

async def main(shield_scope2):
    print(f'** {shield_scope2=}')
    with CancelScope(shield=False) as scope1:
        with CancelScope(shield=shield_scope2) as scope2:     ##
            with CancelScope(shield=True) as scope3:
                scope1.cancel()
                scope2.cancel()
                print('     -- Cancel completed')

        print(f'{scope1.cancel_called=}')
        print(f'{scope2.cancel_called=}')
        print(f'{scope3.cancel_called=}')

        print(f'{scope1.shield=}')
        print(f'{scope2.shield=}')
        print(f'{scope3.shield=}')
              
        await sleep(0)
        print('!! Unshielded')

run(main, True); print()
run(main, False); print()

Outcome:

** shield_scope2=True
     -- Cancel completed
scope1.cancel_called=True
scope2.cancel_called=True
scope3.cancel_called=False
scope1.shield=False
scope2.shield=True
scope3.shield=True

** shield_scope2=False
     -- Cancel completed
scope1.cancel_called=True
scope2.cancel_called=True
scope3.cancel_called=False
scope1.shield=False
scope2.shield=False
scope3.shield=True
!! Unshielded

I would expect no difference in behaviour when scope2 is either shielded or unshielded. The line with "!! Unshielded" should not be reached in either case.

@agronholm agronholm added asyncio Involves the asyncio backend bug Something isn't working labels Sep 18, 2021
@agronholm
Copy link
Owner

It only prints !! Unshielded on the asyncio backend. I will need to look into this. Even setting the sleep period to 1 second did not result in cancellation, so something is off in the task auto-cancellation system of the asyncio backend.

@agronholm
Copy link
Owner

Another curious thing is that it really does need those three levels of cancel scopes to repro the bug.

@agronholm
Copy link
Owner

I turned this into a regression test that currently fails on asyncio but passes on trio:

async def test_triple_nested_shield() -> None:
    """Regression test for #370."""

    got_past_checkpoint = False

    async def taskfunc() -> None:
        nonlocal got_past_checkpoint

        with CancelScope() as scope1:
            with CancelScope() as scope2:
                with CancelScope(shield=True):
                    scope1.cancel()
                    scope2.cancel()

            await checkpoint()
            got_past_checkpoint = True

    async with create_task_group() as tg:
        tg.start_soon(taskfunc)

    assert not got_past_checkpoint

@agronholm
Copy link
Owner

I think the gist of the issue is in the combination of _deliver_cancellation_to_parent() and __exit__() methods of CancelScope. The docstring of the former says Start cancellation effort in the nearest directly cancelled parent scope. What happens when the third scope is exited is that the cancellation effort is restarted in scope2 but when that is exited right after, the cancellation effort is not restarted in scope1 because the condition in __exit__ is that the closing scope was shielded. So clearly there is a flaw in the logic here. However, my initial attempts to fix this caused other tests to fail and I'm still investigating.

@agronholm
Copy link
Owner

I've released v3.3.2 with the fix for this issue.

@arthurxlee
Copy link
Author

I am using the new release in my project and it is running as expected. Big thank you for your effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
asyncio Involves the asyncio backend bug Something isn't working
Projects
None yet
2 participants