Potential segmentation fault when subtyping greenlet

[fygao-wish]

Hi,

I recently ran into some weird errors when running an application with subtyped greenlet.
Some example error messages:

• Fatal Python error: GC object already tracked
• AttributeError: run
• Segmentation fault

After some digging, I found the root cause is that when a greenlet is resurrected, it keeps the object alive, but the subtype is decrefed. Similar issue here: https://bugs.python.org/issue8212

Here's a snippet to repro the problem:

from greenlet import getcurrent, greenlet
from concurrent.futures import ThreadPoolExecutor
import sys
import time

class MyGreenlet(greenlet):
    def __init__(self, *args, **kwargs):
        super(MyGreenlet, self).__init__(*args, **kwargs)

    def switch(self):
        super(MyGreenlet, self).switch()

TPOOL = ThreadPoolExecutor(max_workers=1)
# In main thread, we start greenlets and save them in the glets array
# In order to trigger greenlet resurrect easily, we start another thread to clear the glets array
glets = []

def clean_greenlets():
    while True:
        time.sleep(1)
        if glets:
            glets.clear()

# start a thread to clear the glets list
TPOOL.submit(clean_greenlets)

def gr_run():
    while True:
        time.sleep(0.10)
        print("{}".format(sys.getrefcount(MyGreenlet)))
        getcurrent().parent.switch()

while True:
    glet = MyGreenlet(gr_run)
    glets.append(glet)
    glet.switch()

[jamadden]

This turns out to be quite a kettle of fish, and it's not exactly "resurrection" as considered from the Python level — you can't provoke this with __del__. Here's what's going on.

If the last reference to a greenlet goes away while the greenlet is still running, then a GreenletExit exception is raised in the greenlet so that it can do cleanup (execute finally blocks, or __exit__ of context managers, deallocate local variables, etc). So far so good.

However, if some other thread besides the thread the greenlet belongs to (runs in) is the thread that deletes that final reference, we can't raise the GreenletExit in the soon-to-die greenlet: Just as you can't switch() to a greenlet on another thread, you can't raise an exception in a greenlet on another thread.

So to workaround this, when we discover this situation, greenlet internally "resurrects" the dyeing greenlet into a thread-local list, local to the list running the greenlet and calls it good. It is no longer reachable from Python, and we'll actually handle the cleanup at sometime in the future.

That time in the future is when the dyeing greenlet's thread is again running, and somebody calls one of several greenlet APIs — getcurrent() being the one used here. Every time those APIs are called, greenlet checks the thread-local list to see if there are any suspended greenlets waiting to have their GreenletExit exception raised, and if so, raises that in what is now the correct thread.

This approach has some bugs and issues.

The first is the reference count issue with the type, as you discovered. Here's a simplified example that doesn't involve any nesting or indeterminate sleeps:

from __future__ import print_function
import sys

from threading import Thread
from threading import Event

from greenlet import getcurrent, greenlet


class MyGreenlet(greenlet):
    pass


glets = []
ref_cleared = Event()

def greenlet_main():
    getcurrent().parent.switch()

def thread_main(greenlet_running_event):
    mine = MyGreenlet(greenlet_main)
    glets.append(mine)
    # The greenlets being deleted must be active
    mine.switch()
    # Don't keep any reference to it in this thread
    del mine
    # Let main know we published our greenlet.
    greenlet_running_event.set()
    # Wait for main to let us know the references are
    # gone and the greenlet objects no longer reachable
    ref_cleared.wait()
    # The creating thread must call getcurrent() (or a few other
    # greenlet APIs) because that's when the thread-local list of dead
    # greenlets gets cleared.
    getcurrent()

# We start with 3 references to the subclass:
# - This module
# - Its __mro__
# - The __subclassess__ attribute of greenlet
# - (If we call gc.get_referents(), we find four entries, including
#   some other tuple ``(greenlet)`` that I'm not sure about but must be part
#   of the machinery.)
#
# On Python 3 it's enough to just run 3 threads; on Python 2.7,
# more threads are needed, and the results are still
# non-deterministic. Presumably the memory layouts are different
thread_ready_events = []
for _ in range(
        sys.getrefcount(MyGreenlet) + (
            15 if sys.version_info[0] < 3 else 0)
):
    event = Event()
    thread = Thread(target=thread_main, args=(event,))
    thread_ready_events.append(event)
    thread.start()


for done_event in thread_ready_events:
    done_event.wait()


del glets[:]
ref_cleared.set()
# Let any other thread run; it will crash the interpreter. A
# ``time.sleep()`` will also work.
with open('/dev/null', 'wb') as f:
    f.write(b'foobar')

The second is that there is an extra reference to that thread-local list, preventing it from ever being deallocated. This means that any time we'd need to delete a greenlet in another thread, we at least leak one list object when that thread exits. That's a straightforward bug to fix.

The third is that if the thread that the dying greenlet is running in doesn't call any of the greenlet APIs that perform this "garbage collection" before it exits (and after the last reference was cleared by some other thread), the to-be-killed greenlets are left in limbo and never deallocated, nor is anything they reference through their attributes, on their stacks, etc. They stay in that list indefinitely, and the list is leaked. This is true even if the above bug is fixed, because there is a reference cycle (thread locals → dead greenlet list → a greenlet → thread locals), and, even though greenlet objects participate in GC and visit the thread locals, they only do this when they are the actively running greenlet. The suspended greenlets in the dead greenlet list thus get skipped over.

[jamadden]

The type reference counting issue can also be reproduce independently of the thread-local list, or indeed, threads at all. This causes a crash (reliably on Python 3.10, less reliably on earlier versions):

from greenlet import getcurrent
from greenlet import greenlet
from greenlet import GreenletExit

class Greenlet(greenlet):
    pass

glets = []

def greenlet_main():
    try:
        getcurrent().parent.switch()
    except GreenletExit:
        glets.append(getcurrent())


for _ in range(10):
    Greenlet(greenlet_main).switch()

del glets

[jamadden]

Thanks for opening this issue and the PR!

The crash is fixed. The remaining things this investigation revealed and which still need addressed have been moved to #251 and #252.