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

Exception state lives too long in 3.0 #46759

Closed
pitrou opened this issue Mar 29, 2008 · 18 comments
Closed

Exception state lives too long in 3.0 #46759

pitrou opened this issue Mar 29, 2008 · 18 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@pitrou
Copy link
Member

pitrou commented Mar 29, 2008

BPO 2507
Nosy @warsaw, @pitrou, @benjaminp
Superseder
  • bpo-3021: Lexical exception handlers
  • Files
  • exc_cleanup.patch
  • exc_cleanup2.patch
  • exc_cleanup3.patch
  • exc_cleanup4.patch
  • better_exc_cleanup.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 2008-06-11.16:04:10.339>
    created_at = <Date 2008-03-29.14:28:37.428>
    labels = ['interpreter-core', 'type-bug']
    title = 'Exception state lives too long in 3.0'
    updated_at = <Date 2008-06-11.16:04:10.277>
    user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2008-06-11.16:04:10.277>
    actor = 'benjamin.peterson'
    assignee = 'none'
    closed = True
    closed_date = <Date 2008-06-11.16:04:10.339>
    closer = 'benjamin.peterson'
    components = ['Interpreter Core']
    creation = <Date 2008-03-29.14:28:37.428>
    creator = 'pitrou'
    dependencies = []
    files = ['9889', '9898', '10171', '10213', '10430']
    hgrepos = []
    issue_num = 2507
    keywords = ['patch']
    message_count = 18.0
    messages = ['64697', '64698', '64734', '64736', '64883', '66108', '66344', '66353', '66379', '66403', '66793', '67291', '67293', '67296', '67301', '67311', '67599', '67990']
    nosy_count = 6.0
    nosy_names = ['barry', 'collinwinter', 'Rhamphoryncus', 'pitrou', 'jyasskin', 'benjamin.peterson']
    pr_nums = []
    priority = 'critical'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = '3021'
    type = 'behavior'
    url = 'https://bugs.python.org/issue2507'
    versions = ['Python 3.0']

    @pitrou
    Copy link
    Member Author

    pitrou commented Mar 29, 2008

    See http://mail.python.org/pipermail/python-3000/2008-March/012830.html
    : the exception state can survive across thread switches if the
    enclosing frame is not destroyed immediately.

    @pitrou pitrou added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Mar 29, 2008
    @pitrou
    Copy link
    Member Author

    pitrou commented Mar 29, 2008

    And this is a patch, together with a test.

    @jyasskin
    Copy link
    Mannequin

    jyasskin mannequin commented Mar 29, 2008

    Thanks for the patch. This isn't specific to threads at all, so the test
    doesn't need to spawn a thread, just raise an exception from a nested
    function with a parameter, catch it, delete the object the parameter
    referred to, and then check that the object has been destroyed.

    I don't know the exception handling code very well, so I'd like someone
    more familiar with it to check that the patch is correct and efficient.
    Collin, would you do the honors?

    @jyasskin jyasskin mannequin assigned collinwinter Mar 29, 2008
    @pitrou
    Copy link
    Member Author

    pitrou commented Mar 29, 2008

    You are right, threads aren't needed. So, attaching an updated patch.

    @warsaw
    Copy link
    Member

    warsaw commented Apr 2, 2008

    Knocking the priority down to critical for the next alpha release. This
    can be readdressed once the release is made.

    @pitrou
    Copy link
    Member Author

    pitrou commented May 2, 2008

    A new patch with just an updated comment in ceval.c.

    @collinwinter
    Copy link
    Mannequin

    collinwinter mannequin commented May 6, 2008

    Looks good to me. Antoine, do you have commit privileges?

    @pitrou
    Copy link
    Member Author

    pitrou commented May 7, 2008

    No, I don't.

    @pitrou
    Copy link
    Member Author

    pitrou commented May 7, 2008

    Here is a last patch fixing without lines longer than 78 chars.

    @warsaw
    Copy link
    Member

    warsaw commented May 8, 2008

    r62847

    @pitrou
    Copy link
    Member Author

    pitrou commented May 13, 2008

    This bug should be reopened, the patch does not fix it when the except
    clause does not assign the exception to a local variable (that is,
    "except KeyError" rather than "except KeyError as e").
    Another can of worms also appeared in bpo-2833...

    @collinwinter collinwinter mannequin reopened this May 13, 2008
    @collinwinter collinwinter mannequin removed their assignment May 13, 2008
    @pitrou
    Copy link
    Member Author

    pitrou commented May 24, 2008

    This is a complementary patch that should fix the remaining cases
    (funnily doctest was relying on the old behaviour).
    Unfortunately bpo-2833 still remains, and would probably need some
    discussion on the ML for proper resolution.

    @benjaminp
    Copy link
    Contributor

    For time being, should r62847 be reverted?

    @pitrou
    Copy link
    Member Author

    pitrou commented May 24, 2008

    I don't think so. As Amaury pointed in bpo-2833, the original re-raising
    behaviour which got changed by fixing the present bug was rather broken
    in its own way.
    Anyway, I think the question better be asked on the ML because any
    solution for the re-raising behaviour will require some significant
    trickery in the compiler and the eval loop.

    @pitrou
    Copy link
    Member Author

    pitrou commented May 24, 2008

    Patch removed, there are still some unsolved cases :-)

    @pitrou
    Copy link
    Member Author

    pitrou commented May 24, 2008

    Here is a hopefully complete patch using the same approach as before.

    However I'm now convinced that another approach would be better in order
    to solve both this bug and the various re-raising issues. It would
    involve replacing most of the set_exc_info() / reset_exc_info()
    mechanism in ceval.c with something different, probably based on
    saving/restoring exception state values onto/from the frame stack.
    Problem is, I'm not sure I'll have time for it very soon.

    @pitrou
    Copy link
    Member Author

    pitrou commented Jun 1, 2008

    A clean solution is now proposed in bpo-3021.

    @benjaminp
    Copy link
    Contributor

    Solved by r64121.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    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) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants