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

gc.get_referrers() is inherently dangerous #39117

Closed
arigo mannequin opened this issue Aug 23, 2003 · 8 comments
Closed

gc.get_referrers() is inherently dangerous #39117

arigo mannequin opened this issue Aug 23, 2003 · 8 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@arigo
Copy link
Mannequin

arigo mannequin commented Aug 23, 2003

BPO 793822
Nosy @tim-one, @loewis, @brettcannon, @arigo, @ods
Files
  • tupletest.py
  • libgc.diff: documentation patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2003-10-28.12:11:38.000>
    created_at = <Date 2003-08-23.17:17:54.000>
    labels = ['interpreter-core']
    title = 'gc.get_referrers() is inherently dangerous'
    updated_at = <Date 2003-10-28.12:11:38.000>
    user = 'https://github.com/arigo'

    bugs.python.org fields:

    activity = <Date 2003-10-28.12:11:38.000>
    actor = 'arigo'
    assignee = 'none'
    closed = True
    closed_date = None
    closer = None
    components = ['Interpreter Core']
    creation = <Date 2003-08-23.17:17:54.000>
    creator = 'arigo'
    dependencies = []
    files = ['1007', '1008']
    hgrepos = []
    issue_num = 793822
    keywords = []
    message_count = 8.0
    messages = ['17902', '17903', '17904', '17905', '17906', '17907', '17908', '17909']
    nosy_count = 5.0
    nosy_names = ['tim.peters', 'loewis', 'brett.cannon', 'arigo', 'ods']
    pr_nums = []
    priority = 'normal'
    resolution = 'accepted'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue793822'
    versions = ['Python 2.3']

    @arigo
    Copy link
    Mannequin Author

    arigo mannequin commented Aug 23, 2003

    gc.get_referrers() can be used to crash any Python
    interpreter because it allows the user to obtain
    objects which are still under construction.

    Here is an example where an iterator can use it to
    obtain a tuple while it is still being populated by the
    'tuple' built-in function. The following example
    triggers a SystemError, but as the tuple 't' is at the
    moment still full of null values it can easily generate
    segfaults instead.

    from gc import get_referrers
    
    def iter():
        tag = object()
        yield tag   # 'tag' gets stored in the result tuple
        lst = [x for x in get_referrers(tag)
               if isinstance(x, tuple)]
        t = lst[0]  # this *is* the result tuple
        print t     # full of nulls !
    
    tuple(iter())

    Unless someone has more ideas than me as to how to
    prevent this problem, I'd suggest that
    gc.get_referrers() should be deemed 'officially
    dangerous' in the docs.

    @arigo arigo mannequin closed this as completed Aug 23, 2003
    @arigo arigo mannequin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Aug 23, 2003
    @arigo arigo mannequin closed this as completed Aug 23, 2003
    @arigo arigo mannequin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Aug 23, 2003
    @ods
    Copy link
    Mannequin

    ods mannequin commented Aug 28, 2003

    Logged In: YES
    user_id=63454

    I guess it's dangerous to make object that is not properly
    initialized reachable from other code. Even if it's reachable
    with get_referrers() only. There is no danger in
    get_referrers() itself.

    @arigo
    Copy link
    Mannequin Author

    arigo mannequin commented Aug 28, 2003

    Logged In: YES
    user_id=4771

    But it would be very difficult to fix the code base to avoid
    the problem. The 'tuple' constructor was only an example; it
    is actually a quite common pattern everywhere in the C code
    base of both the core and extension modules. Expecting an
    object not to be seen before you first hand it out is
    extremely common, and get_referrers() breaks that
    assumption. Hence the claim that the problem really lies in
    get_referrers().

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Aug 31, 2003

    Logged In: YES
    user_id=21627

    I see no harm in your example. Even though the tuple has
    NULLs in it, is in a stable state. The lesson learned is
    that an object should become gc-tracked only if it is in a
    stable state, as gc-tracking means to expose references to
    the object. This is true independent of get_referrers, as
    gc-tracking means that tp_traverse might be called for the
    object, so it *has* to be in a stable state.

    I fail to see how the example "crashes" the Python
    interpreter. I causes a SystemError when the tuple is
    resized, that's all. There are many ways to cause a
    SystemError, including

    raise SystemError

    I recommend not to declare a function as "dangerous" in the
    docs. Instead, the actual problem should be explained, both
    in the GC part of the C API, and in gc module docs (for both
    get_referrers, and get_objects).

    @tim-one
    Copy link
    Member

    tim-one commented Aug 31, 2003

    Logged In: YES
    user_id=31435

    Martin, it's easy to transform the example into one that
    crashes. For example, adding "print t[3]" as the last line of
    the iter() function causes it to die with a segfault on my box.
    Virtually anything that fetches a NULL from the tuple will try
    to Py_INCREF it (that's all "t[3]" does), and that's an instant
    NULL-pointer dereferencing death.

    That said, it would be more helpful <wink> if Armin submitted
    a doc patch. The introspective features of the gc module are
    intended to be used by consenting adults facing hard
    debugging problems, and I don't care if they can be tricked
    into blowing up. I agree the docs should point out the
    possibility, though.

    @arigo
    Copy link
    Mannequin Author

    arigo mannequin commented Sep 2, 2003

    Logged In: YES
    user_id=4771

    Here is the doc patch (attached).

    A clean solution to this problem would involve delaying GC
    registration, which I think would imply changes in the C
    API. It is probably not worth it. I don't think it would be
    a problem for the GC not to be able to browse through
    objects still under constructions because such objects are
    never part of a dead cycle anyway, and probably not part of
    a cycle at all until later mutated.

    @brettcannon
    Copy link
    Member

    Logged In: YES
    user_id=357491

    The wording Armin proposes in his patch makes sense to me.

    @arigo
    Copy link
    Mannequin Author

    arigo mannequin commented Oct 28, 2003

    Logged In: YES
    user_id=4771

    Checked in:

    Doc/lib/libgc.tex (rev: 1.15)

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants