Skip to content

Commit ea6f77f

Browse files
Thrameosclaude
andcommitted
Rework Java-backed object layout: fixed offsets, fungible boxed values
Replaces the runtime allocator that re-derived each Java-backed Python object's layout from version-sensitive CPython internals (reading private PyLongObject fields on every single value access) with fixed, type-baked offsets resolved once at type-creation time. The abstract/concrete type split makes this offset a hard invariant rather than something callers branch on or recompute. Builds on that to eliminate per-instance Java-value storage entirely for the boxed Long/Boolean/Character wrapper family: their JPClass* moves onto the shared metaclass (redundant per instance otherwise), and their jvalue is reconstructed on demand from the instance's own native representation -- boxing via a real JNI call only when a boxed wrapper's value actually crosses back into Java. This also removes a version-gated digit-layout workaround that existed solely to keep a trailing value at a fixed offset inside a variable-length PyLongObject subtype, along with a related heap corruption bug it caused on Python 3.8-3.11 when combined with an implicit __dict__ slot. A per-leaf-class null-boxed singleton represents JObject(None, cls), since a genuinely null Java reference has no native Python representation to reconstruct from. No user-visible API change. Boxed wrapper instances no longer retain Java-side reference identity across repeated round-trips through Python -- this was never guaranteed for freshly-converted values either, so the change is specifically about previously-received-then-passed-back boxed objects. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent ce88717 commit ea6f77f

29 files changed

Lines changed: 1154 additions & 487 deletions

doc/CHANGELOG.rst

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,25 @@ Latest Changes:
77

88
- **1.7.2.dev0**
99

10+
- Reworked the internal object layout for Java-backed Python objects to use
11+
fixed, type-baked offsets instead of a runtime allocator that re-derived
12+
each object's layout from version-sensitive CPython internals on every
13+
access. For the boxed `Long`/`Boolean`/`Character` wrapper types this
14+
also removes their per-instance Java-value storage entirely (reconstructed
15+
on demand instead), shrinking those instances and eliminating a
16+
version-gated digit-layout workaround. No user-visible API change; boxed
17+
wrapper instances no longer retain Java-side reference identity across
18+
repeated round-trips through Python.
19+
20+
- Fixed heap corruption when boxing large `JLong`/`JInt`/`JShort`/`JByte`/`JBoolean`
21+
values on Python 3.8-3.11, caused by a fixed-offset allocator layout
22+
assumption colliding with CPython's own implicit `__dict__` slot for
23+
variable-length int subclasses.
24+
25+
- Fixed a GC refcount-accounting bug in the internal Java-class metaclass
26+
where `tp_traverse`/`tp_clear` did not chain to `type`'s own
27+
implementation.
28+
1029
- Fixed memory leak with int and float conversions. #1379
1130

1231
- Fixed instablity in threading for method dispatch. #1366

native/common/include/jp_match.h

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,23 +36,41 @@ class JPMatch
3636
JPMatch(JPJavaFrame *frame, PyObject *object);
3737

3838
/**
39-
* Get the Java slot associated with the Python object.
39+
* Get the JPClass associated with the Python object, if any.
4040
*
41-
* Thus uses caching.
41+
* Cached alongside getJValue() -- both are resolved together on first
42+
* use since some JPConversion::matches() implementations (e.g.
43+
* JPConversionUnbox) read the cached jvalue directly, relying on an
44+
* earlier matches() call on the same argument having already resolved
45+
* it.
4246
*
43-
* @return the Java slot or 0 if not available.
47+
* @return the class, or nullptr if not a Java value.
4448
*/
45-
JPValue *getJavaSlot();
49+
JPClass *getJPClass();
50+
51+
/**
52+
* Get the jvalue associated with the Python object.
53+
*
54+
* Only meaningful once getJPClass() is non-null.
55+
*/
56+
jvalue getJValue();
4657

4758
jvalue convert();
4859

60+
private:
61+
void resolveSlot();
62+
4963
public:
5064
JPMatch::Type type;
5165
JPConversion *conversion;
5266
JPJavaFrame *frame;
5367
PyObject *object;
54-
JPValue *slot;
5568
void *closure;
69+
70+
private:
71+
bool m_SlotResolved;
72+
JPClass *m_SlotClass;
73+
jvalue m_SlotValue;
5674
} ;
5775

5876
class JPMethodMatch

native/common/include/jp_primitive_accessor.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -251,10 +251,9 @@ class JPConversionLongWiden : public JPConversion
251251

252252
jvalue convert(JPMatch &match) override
253253
{
254-
JPValue *value = match.getJavaSlot();
255254
jvalue ret;
256255
base_t::field(ret) = (typename base_t::type_t) (dynamic_cast<JPPrimitiveType*>(
257-
value->getClass()))->getAsLong(value->getValue());
256+
match.getJPClass()))->getAsLong(match.getJValue());
258257
return ret;
259258
}
260259
} ;
@@ -331,9 +330,8 @@ class JPConversionFloatWiden : public JPConversion
331330

332331
jvalue convert(JPMatch &match) override
333332
{
334-
JPValue *value = match.getJavaSlot();
335333
jvalue ret;
336-
base_t::field(ret) = (typename base_t::type_t) (dynamic_cast<JPPrimitiveType*>( value->getClass()))->getAsDouble(value->getValue());
334+
base_t::field(ret) = (typename base_t::type_t) (dynamic_cast<JPPrimitiveType*>( match.getJPClass()))->getAsDouble(match.getJValue());
337335
return ret;
338336
}
339337
} ;

native/common/jp_booleantype.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,7 @@ class JPConversionAsBooleanJBool : public JPConversionJavaValue
8585
JPMatch::Type matches(JPClass *cls, JPMatch &match) override
8686
{
8787

88-
JPValue *value = match.getJavaSlot();
89-
if (value == nullptr)
88+
if (match.getJPClass() == nullptr)
9089
return match.type = JPMatch::_none;
9190
match.type = JPMatch::_none;
9291
// Implied conversion from boxed to primitive (JLS 5.1.8)

native/common/jp_boxedtype.cpp

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,16 +116,39 @@ JPPyObject JPBoxedType::convertToPythonObject(JPJavaFrame& frame, jvalue value,
116116
}
117117

118118
JPPyObject wrapper = PyJPClass_create(frame, cls);
119-
JPPyObject obj;
119+
auto *wrapperType = (PyTypeObject*) wrapper.get();
120120
JPContext *context = JPContext_global;
121+
122+
// Reconstructing families (Long/Boolean/Char) keep no per-instance Java
123+
// value at all -- a real Java null needs a dedicated singleton instance
124+
// instead, since there's no slot left on an ordinary instance to mark
125+
// "this one's null". Built once per class and cached; Double/Float (no
126+
// tp_jvalue yet) fall through to the ordinary path below, unchanged.
127+
if (cast && value.l == nullptr && PyJPClass_GetJValueFn(wrapperType) != nullptr)
128+
{
129+
PyObject *nullBoxed = PyJPClass_GetNullBoxed(wrapperType);
130+
if (nullBoxed == nullptr)
131+
{
132+
JPPyObject built;
133+
if (this->getPrimitive() == context->_char)
134+
built = JPPyObject::call(PyJPChar_Create(wrapperType, 0));
135+
else
136+
built = PyJPNumber_create(frame, wrapper, JPValue(cls, value));
137+
PyJPClass_SetNullBoxed(wrapperType, built.get());
138+
nullBoxed = built.get();
139+
}
140+
return JPPyObject::use(nullBoxed);
141+
}
142+
143+
JPPyObject obj;
121144
if (this->getPrimitive() == context->_char)
122145
{
123146
jchar value2 = 0;
124147
// Not null get the char value
125148
if (value.l != nullptr)
126149
value2 = context->_char->getValueFromObject(frame, JPValue(this, value)).getValue().c;
127150
// Create a char string object
128-
obj = JPPyObject::call(PyJPChar_Create((PyTypeObject*) wrapper.get(), value2));
151+
obj = JPPyObject::call(PyJPChar_Create(wrapperType, value2));
129152
} else
130153
obj = PyJPNumber_create(frame, wrapper, JPValue(cls, value));
131154
PyJPValue_assignJavaSlot(frame, obj.get(), JPValue(cls, value));

native/common/jp_bytetype.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ class JPConversionJByte : public JPConversionJavaValue
5858

5959
JPMatch::Type matches(JPClass *cls, JPMatch &match) override
6060
{
61-
JPValue *value = match.getJavaSlot();
62-
if (value == nullptr)
61+
if (match.getJPClass() == nullptr)
6362
return match.type = JPMatch::_none;
6463
match.type = JPMatch::_none;
6564
// Implied conversion from boxed to primitive (JLS 5.1.8)

native/common/jp_chartype.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,7 @@ class JPConversionAsJChar : public JPConversionJavaValue
100100

101101
JPMatch::Type matches(JPClass *cls, JPMatch &match) override
102102
{
103-
JPValue *value = match.getJavaSlot();
104-
if (value == nullptr)
103+
if (match.getJPClass() == nullptr)
105104
return match.type = JPMatch::_none;
106105
match.type = JPMatch::_none;
107106

native/common/jp_class.cpp

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,9 +376,31 @@ JPPyObject JPClass::convertToPythonObject(JPJavaFrame& frame, jvalue value, bool
376376
} else
377377
{
378378
PyTypeObject *type = ((PyTypeObject*) wrapper.get());
379-
// Simple objects don't have a new or init function
380-
PyObject *obj2 = type->tp_alloc(type, 0);
379+
// Simple objects don't have a new or init function. If this type is
380+
// abstract (kept layout-trivial so it can be mixed into any foreign
381+
// family -- see PyJPClass_FromSpecWithBases), redirect the actual
382+
// allocation to its hidden concrete companion, exactly as
383+
// PyJPObject_new does for the ordinary constructor path. offset is
384+
// no longer usable to detect abstract-ness (see PyJPObject_new's
385+
// comment) -- tp_concrete is the real signal.
386+
PyTypeObject *allocType = PyJPClass_getConcrete(type);
387+
if (allocType == nullptr)
388+
allocType = type;
389+
PyObject *obj2 = allocType->tp_alloc(allocType, 0);
381390
JP_PY_CHECK();
391+
392+
if (allocType != type)
393+
{
394+
// Polymorph back to the canonical/abstract type, so
395+
// type(instance) stays consistent with cls->getHost() and any
396+
// other identity-sensitive code -- mirrors PyJPObject_new's own
397+
// polymorph-back (see pyjp_object.cpp) and the legacy
398+
// PyJPValue_alloc's Py_SET_TYPE trick.
399+
Py_INCREF(type);
400+
Py_SET_TYPE(obj2, type);
401+
Py_DECREF(allocType);
402+
}
403+
382404
obj = JPPyObject::claim(obj2);
383405
}
384406

native/common/jp_classhints.cpp

Lines changed: 47 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -25,18 +25,35 @@
2525
#include "pyjp.h"
2626

2727
JPMatch::JPMatch() : conversion(nullptr), frame(nullptr), object(nullptr),
28-
type(JPMatch::_none), slot((JPValue*) - 1), closure(nullptr)
28+
type(JPMatch::_none), closure(nullptr),
29+
m_SlotResolved(false), m_SlotClass(nullptr), m_SlotValue()
2930
{}
3031

3132
JPMatch::JPMatch(JPJavaFrame *fr, PyObject *obj) : conversion(nullptr), frame(fr), object(obj),
32-
type(JPMatch::_none), slot((JPValue*) - 1), closure(nullptr)
33+
type(JPMatch::_none), closure(nullptr),
34+
m_SlotResolved(false), m_SlotClass(nullptr), m_SlotValue()
3335
{}
3436

35-
JPValue *JPMatch::getJavaSlot()
37+
void JPMatch::resolveSlot()
3638
{
37-
if (slot == (JPValue*) - 1)
38-
return slot = PyJPValue_getJavaSlot(object);
39-
return slot;
39+
if (m_SlotResolved)
40+
return;
41+
m_SlotResolved = true;
42+
m_SlotClass = PyJPValue_getJPClass(object);
43+
if (m_SlotClass != nullptr && frame != nullptr)
44+
m_SlotValue = PyJPValue_getJValue(*frame, object);
45+
}
46+
47+
JPClass *JPMatch::getJPClass()
48+
{
49+
resolveSlot();
50+
return m_SlotClass;
51+
}
52+
53+
jvalue JPMatch::getJValue()
54+
{
55+
resolveSlot();
56+
return m_SlotValue;
4057
}
4158

4259
jvalue JPMatch::convert()
@@ -150,10 +167,10 @@ class JPPythonConversion : public JPConversion
150167
JPClass *cls = ((JPClass*) match.closure);
151168
JPPyObject args = JPPyTuple_Pack(cls->getHost(), match.object);
152169
JPPyObject ret = JPPyObject::call(PyObject_Call(method_.get(), args.get(), nullptr));
153-
JPValue *value = PyJPValue_getJavaSlot(ret.get());
154-
if (value != nullptr)
170+
JPClass *retCls = PyJPValue_getJPClass(ret.get());
171+
if (retCls != nullptr)
155172
{
156-
jvalue v = value->getValue();
173+
jvalue v = PyJPValue_getJValue(*match.frame, ret.get());
157174
JP_TRACE("Value", v.l);
158175
v.l = match.frame->NewLocalRef(v.l);
159176
return v;
@@ -618,13 +635,10 @@ class JPConversionObject : public JPConversion
618635
JPMatch::Type matches(JPClass *cls, JPMatch &match) override
619636
{
620637
JP_TRACE_IN("JPConversionObject::matches");
621-
JPValue *value = match.getJavaSlot();
622-
if (value == nullptr || match.frame == nullptr)
638+
JPClass *oc = match.getJPClass();
639+
if (oc == nullptr || match.frame == nullptr)
623640
return match.type = JPMatch::_none;
624641
match.conversion = this;
625-
JPClass *oc = value->getClass();
626-
if (oc == nullptr)
627-
return match.type = JPMatch::_none;
628642
if (oc == cls)
629643
{
630644
// hey, this is me! :)
@@ -660,17 +674,16 @@ class JPConversionObject : public JPConversion
660674
jvalue convert(JPMatch &match) override
661675
{
662676
jvalue res;
663-
JPValue* value = match.getJavaSlot();
664-
res.l = match.frame->NewLocalRef(value->getValue().l);
677+
res.l = match.frame->NewLocalRef(match.getJValue().l);
665678
return res;
666679
}
667680
} _objectConversion;
668681

669682
JPMatch::Type JPConversionJavaValue::matches(JPClass *cls, JPMatch &match)
670683
{
671684
JP_TRACE_IN("JPConversionJavaValue::matches");
672-
JPValue *slot = match.getJavaSlot();
673-
if (slot == nullptr || slot->getClass() != cls)
685+
JPClass *oc = match.getJPClass();
686+
if (oc == nullptr || oc != cls)
674687
return match.type = JPMatch::_none;
675688
match.conversion = this;
676689
return match.type = JPMatch::_exact;
@@ -685,8 +698,7 @@ void JPConversionJavaValue::getInfo(JPClass *cls, JPConversionInfo &info)
685698

686699
jvalue JPConversionJavaValue::convert(JPMatch &match)
687700
{
688-
JPValue* value = match.getJavaSlot();
689-
return *value;
701+
return match.getJValue();
690702
}
691703

692704
JPConversionJavaValue _javaValueConversion;
@@ -851,13 +863,13 @@ class JPConversionJavaObjectAny : public JPConversionBox
851863
JPMatch::Type matches(JPClass *cls, JPMatch &match) override
852864
{
853865
JP_TRACE_IN("JPConversionJavaObjectAny::matches");
854-
JPValue *value = match.getJavaSlot();
855-
if (value == nullptr || match.frame == nullptr || value->getClass() == nullptr)
866+
JPClass *oc = match.getJPClass();
867+
if (oc == nullptr || match.frame == nullptr)
856868
return match.type = JPMatch::_none;
857869
match.conversion = this;
858-
if (value->getClass()->isPrimitive())
870+
if (oc->isPrimitive())
859871
match.type = JPMatch::_implicit;
860-
else if (value->getClass() == cls)
872+
else if (oc == cls)
861873
match.type = JPMatch::_exact;
862874
else
863875
match.type = JPMatch::_derived;
@@ -875,15 +887,15 @@ class JPConversionJavaObjectAny : public JPConversionBox
875887
{
876888
jvalue res;
877889
JPJavaFrame *frame = match.frame;
878-
JPValue *value = match.getJavaSlot();
879-
if (!value->getClass()->isPrimitive())
890+
JPClass *oc = match.getJPClass();
891+
if (!oc->isPrimitive())
880892
{
881-
res.l = frame->NewLocalRef(value->getJavaObject());
893+
res.l = frame->NewLocalRef(match.getJValue().l);
882894
return res;
883895
} else
884896
{
885897
// Okay we need to box it.
886-
auto* type = dynamic_cast<JPPrimitiveType*> (value->getClass());
898+
auto* type = dynamic_cast<JPPrimitiveType*> (oc);
887899
match.closure = type->getBoxedClass(*frame);
888900
res = JPConversionBox::convert(match);
889901
return res;
@@ -899,15 +911,14 @@ class JPConversionJavaNumberAny : public JPConversionJavaObjectAny
899911
{
900912
JP_TRACE_IN("JPConversionJavaNumberAny::matches");
901913
JPContext *context = JPContext_global;
902-
JPValue *value = match.getJavaSlot();
914+
JPClass *oc = match.getJPClass();
903915
// This converter only works for number types, thus boolean and char
904916
// are excluded.
905-
if (value == nullptr || match.frame == nullptr || value->getClass() == nullptr
906-
|| value->getClass() == context->_boolean
907-
|| value->getClass() == context->_char)
917+
if (oc == nullptr || match.frame == nullptr
918+
|| oc == context->_boolean
919+
|| oc == context->_char)
908920
return match.type = JPMatch::_none;
909921
match.conversion = this;
910-
JPClass *oc = value->getClass();
911922
// If it is the exact type, then it is exact
912923
if (oc == cls)
913924
return match.type = JPMatch::_exact;
@@ -937,9 +948,9 @@ class JPConversionUnbox : public JPConversion
937948
JPContext *context = JPContext_global;
938949
if (context == nullptr)
939950
return match.type = JPMatch::_none;
940-
JPValue *slot = match.slot;
951+
JPClass *oc = match.getJPClass();
941952
auto *pcls = dynamic_cast<JPPrimitiveType*>( cls);
942-
if (slot->getClass() != pcls->getBoxedClass(*match.frame))
953+
if (oc != pcls->getBoxedClass(*match.frame))
943954
return match.type = JPMatch::_none;
944955
match.conversion = this;
945956
match.closure = cls;
@@ -956,9 +967,8 @@ class JPConversionUnbox : public JPConversion
956967

957968
jvalue convert(JPMatch &match) override
958969
{
959-
JPValue* value = match.getJavaSlot();
960970
auto *cls = (JPClass*) match.closure;
961-
return cls->getValueFromObject(*match.frame, *value);
971+
return cls->getValueFromObject(*match.frame, JPValue(match.getJPClass(), match.getJValue()));
962972
}
963973
} _unboxConversion;
964974

0 commit comments

Comments
 (0)