segfault when deleting an object after returning it

[graingert]

a slightly minimized segfault:

import asyncio


async def asyncfn():
    pass

def create_task(loop, coro, **kwargs):
    task = asyncio.Task(coro, loop=loop, **kwargs)
    try:
        return task
    finally:
        # gh-128552: prevent a refcycle of
        # task.exception().__traceback__->BaseEventLoop.create_task->task
        del task

async def main():
    t = create_task(asyncio.get_running_loop(), asyncfn(), eager_start=True, name="example")
    assert t.done()
    await t

asyncio.run(main(), loop_factory=asyncio.EventLoop)

Originally posted by @graingert in #128306 (comment)

Linked PRs

• gh-133371: Don't optimize LOAD_FAST instructions whose local is killed by DELETE_FAST #133383

[efimov-mikhail]

I also have a segfault on this source code snippet.

Moreover, very simplified example like this

def create_obj():
    obj = [42]
    try:
        return obj
    finally:
        del obj

o = create_obj()
print(o)

also have a problem (assertion, not crash):

python: ./Include/internal/pycore_gc.h:167: _PyGCHead_SET_NEXT: Assertion `(unext & ~_PyGC_PREV_MASK) == 0' failed.
[1]    667990 IOT instruction  ./python asyncio_segfault.py

Is it in general correct to delete returned object in finally section?

[graingert]

Is it in general correct to delete returned object in finally section?

Yes, this is correct

[ZeroIntensity]

The returned object is being borrowed, so the del releases the caller's strong reference, not its own.

cc @mpage

[efimov-mikhail]

The returned object is being borrowed, so the del releases the caller's strong reference, not its own.

cc @mpage

Yes, this problem appears after this PR was being merged:
#130708

I've checked it with git bisect.

Another important detail I've noticed is that sys.getrefcount result have changed after this PR.
Maybe this also should be fixed.
Or at least docstring for this method should be adopted.

[ZeroIntensity]

Another important detail I've noticed is that sys.getrefcount result have changed after this PR.

Yeah, there's less strong references being formed, so the reference count isn't as high. Is there anything to document?

[efimov-mikhail]

I've checked that original code snippet with asyncio.Task also works well (no crash) on the last commit before PR #130708.

Yeah, there's less strong references being formed, so the reference count isn't as high. Is there anything to document?

Return the reference count of the object. The count returned is generally one higher than you might expect, because it includes the (temporary) reference as an argument to getrefcount().

Text "one higher than you might expect" is not appropriate for this situation.

[hugovk]

#133383 is merged, can this be closed now?

[graingert]

yes, I believe it was a 3.14 only issue