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

Jump to bottom

invalid result value of _weakref.__init__() #47884

Closed
vstinner opened this issue Aug 21, 2008 · 9 comments
Closed

invalid result value of _weakref.__init__() #47884

vstinner opened this issue Aug 21, 2008 · 9 comments
Assignees
@benjaminp
Labels
stdlib Python modules in the Lib dir

Comments

@vstinner
Copy link
Member

BPO 3634
Nosy @amauryfa, @pitrou, @vstinner, @devdanzin, @benjaminp
Files
  • weakref_init.patch: Fix weakref___init__() return value: -1 on error
  • weakref_test-2.patch: Test .init() using assertRaise()
    		•

    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 = 'https://github.com/benjaminp'
closed_at = <Date 2008-09-09.20:56:33.433>
created_at = <Date 2008-08-21.17:25:00.721>
labels = ['library']
title = 'invalid result value of _weakref.__init__()'
updated_at = <Date 2008-09-09.20:56:33.412>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2008-09-09.20:56:33.412>
actor = 'benjamin.peterson'
assignee = 'benjamin.peterson'
closed = True
closed_date = <Date 2008-09-09.20:56:33.433>
closer = 'benjamin.peterson'
components = ['Library (Lib)']
creation = <Date 2008-08-21.17:25:00.721>
creator = 'vstinner'
dependencies = []
files = ['11195', '11434']
hgrepos = []
issue_num = 3634
keywords = ['patch']
message_count = 9.0
messages = ['71662', '72809', '72811', '72812', '72830', '72833', '72835', '72842', '72909']
nosy_count = 5.0
nosy_names = ['amaury.forgeotdarc', 'pitrou', 'vstinner', 'ajaksu2', 'benjamin.peterson']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue3634'
versions = ['Python 2.6', 'Python 3.0']
    @vstinner
    Copy link
    Member Author

    _weakref.__init__() doesn't catch errors correctly. Example:
    --------------------- 8< -------------------------

    from gc import collect
import _weakref

class FuzzingUserClass:
    pass

obj = _weakref.ref(FuzzingUserClass)

# Exception not raised??
obj.__init__(
    0,
    0,
    0,
)

# Exception catched here??
collect()
--------------------- 8<

    Attached patch fix the bug for py3k branch: return -1 on error
    (instead of 1).

    @vstinner vstinner added the stdlib Python modules in the Lib dir label Aug 21, 2008
    @vstinner
    Copy link
    Member Author

    vstinner commented Sep 8, 2008

    The bug and the fix are trivials. Can anyone review my patch?

    @benjaminp
    Copy link
    Contributor

    The patch looks absolutely fine to me. (I think I have to have another
    core developer look at it too, though.)

    @benjaminp benjaminp self-assigned this Sep 8, 2008
    @pitrou
    Copy link
    Member

    pitrou commented Sep 9, 2008

    Adding a simple unit test would be nice.

    @vstinner
    Copy link
    Member Author

    vstinner commented Sep 9, 2008

    Add a test to check to regression.

    @amauryfa
    Copy link
    Member

    amauryfa commented Sep 9, 2008

    I think the test should check that TypeError is actually raised:
    self.assertRaises(TypeError, r.__init__, 0, 0, 0, 0, 0)
    It's even shorter than the try/except block...

    @vstinner
    Copy link
    Member Author

    vstinner commented Sep 9, 2008

    amaury: oh yes, i forget to use assertRaise(). A new patch is
    attached.

    @amauryfa
    Copy link
    Member

    amauryfa commented Sep 9, 2008

    Both patches look fine to me.
    They could be backported to 2.5 as well.

    @benjaminp
    Copy link
    Contributor

    Fixed in r66352.

    @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
    Assignees

    @benjaminp benjaminp

    Labels
    stdlib Python modules in the Lib dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @vstinner @benjaminp @amauryfa @pitrou