Repository navigation
Conversation
refTypeWithCallback skipped the weaklistoffset check that refType does, so weakref.ref(obj, cb) and weakref.proxy(obj, cb) accepted dicts, None and other builtin objects CPython cannot reference weakly. Add the check, and give the builtin types that CPython does reference weakly but that were missing from getBuiltinTypeWeaklistoffset their CPython 3.13 offsets: deque, array, mmap, Context, SimpleQueue, Struct and the TypeVar family. Without the entries the check would reject them, and today they already fail without a callback. Signed-off-by: Jeongseop Lim <jeongseop_lim@korea.ac.kr>
Each object and constructor gets its own assertion, so a failure names the case, and the mmap is closed after use. Signed-off-by: Jeongseop Lim <jeongseop_lim@korea.ac.kr>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #1256
ReferenceTypeNode.refTypeWithCallback, used byweakref.ref(obj, cb)andweakref.proxy(obj, cb), skipped thegetWeaklistoffset()check thatrefTypedoes for aNonecallback, sodict,list,int,Noneand other builtin objects got a weak reference. CPython checks_PyType_SUPPORTS_WEAKREFSbefore it looks at the callback (weakrefobject.c#L405-L415).getBuiltinTypeWeaklistoffsetwas also missingcollections.deque,array.array,mmap.mmap,_contextvars.Context,_queue.SimpleQueue,_struct.Structand thetyping.TypeVarfamily. These worked only with a callback, so the check alone would have rejected them. With the entries, they now accept a weak reference without a callback too, as on CPython.AS-IS
TO-BE
(matching CPython)
Changes
TypeErrorinrefTypeWithCallbackwhen the object's class is aPythonBuiltinClassTypewith a zerogetWeaklistoffset(), asrefTypedoes.deque,array,mmap,Context,SimpleQueueandStructtogetBuiltinTypeWeaklistoffsetwith their CPython 3.13 offsets, andTypeVar,TypeVarTuple,ParamSpec,ParamSpecArgsandParamSpecKwargswith the managed weakref offset-32.test_weakref_with_callback_object_type_supportandtest_weakref_builtin_types_with_weaklisttotests/test_weakref.py.Testing
mx python-jvmon darwin-aarch64.tests/test_weakref.pypasses (13 tests), and both new tests fail on the unpatched build. The taggedlib-pythonsuitestest_weakref,test_weakset,test_typing,test_deque,test_array,test_mmap,test_queue,test_struct,test_context,test_functools,test_reandtest_threading_localpass. Over 79 builtin, stdlib and user-defined objects,weakref.ref(obj),weakref.ref(obj, cb)andweakref.proxy(obj, cb)now succeed or raiseTypeErrorexactly as on CPython 3.13.14, apart from a__slots__ = ()class andoperator.attrgetter, which GraalPy already accepted without a callback.