Skip to content

Commit 2f1f0b6

Browse files
committed
Fix prepended module initialize not called for Ruby classes rooted in a Java superclass
When a module M is prepended to a Ruby class E that subclasses a Java class, the split constructor protocol only saw M's initialize (the effective method after prepend) and never invoked E's own initialize, producing [:d, :c] instead of the correct [:m, :e, :d, :c]. Root causes: - tryInstall used clazz.searchMethod which found M's initialize (via prepend hierarchy) instead of E's, so oldInit was wrong - splitInitialized had no way to express two Ruby initializes at the same reified-class level (M wrapping E), only a flat isLateral branch - SplitCtorData lacked a clazz field so finishInitialize used getMetaClass() as the defining module, causing incorrect method lookup Fix: - tryInstall now uses clazz.getMethodLocation().searchMethod to capture E's own initialize as oldInit - Add findIncludedPrependedModule helper to detect when the found method lives in an included prepend-wrapper for the base class - Add isPrependedJavaCtorWrapper to identify this specific structural case - In splitInitialized, handle the prepend-wrapper case by splitting M first, recursing into the prepend-wrapper's superclass for E's split, then chaining the two SplitCtorData via a new nested field - finishInitialize recurses through nested first, ensuring M->E->D->C order - Add clazz field to SplitCtorData so finishInitialize uses the correct defining module for direct (no-state) calls - Fix isClassOrIncludedPrependedModule to use getDelegate() comparisons
1 parent b32dc12 commit 2f1f0b6

3 files changed

Lines changed: 136 additions & 16 deletions

File tree

core/src/main/java/org/jruby/java/proxies/ConcreteJavaProxy.java

Lines changed: 64 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule claz
201201
}
202202

203203
}
204-
204+
205205
/**
206206
* Manually added as an override of `new` for Concrete Extension
207207
*/
@@ -221,10 +221,12 @@ public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule claz
221221
IRubyObject[] args, Block block) {
222222
try {
223223
ConcreteJavaProxy cjp = (ConcreteJavaProxy) self;
224-
// TODO: Insead of selectively overwriting, silently fail? or only use the other method/this method?
224+
// TODO: Instead of selectively overwriting, silently fail? or only use the other method/this method?
225225
if (cjp.getObject() == null) {
226226
withBlock.newInstance(cjp, args, block, context.runtime, clazz);
227227
// note: the generated ctor sets self.object = our discarded return of the new object
228+
} else if (oldInit != null) {
229+
return oldInit.call(context, self, clazz, name, args, block);
228230
}
229231
} catch (InstantiationException | InvocationTargetException e) {
230232
throw JavaProxyConstructor.throwInstantiationExceptionCause(context.runtime, e);
@@ -243,7 +245,7 @@ public static void tryInstall(Ruby runtime, RubyClass clazz, JavaProxyClass prox
243245
// TODO: don't lock in this initialize method
244246
var context = runtime.getCurrentContext();
245247
if (overwriteInitialize) clazz.addMethod(context,"initialize",
246-
new StaticJCreateMethod(clazz, withBlock, clazz.searchMethod("initialize")));
248+
new StaticJCreateMethod(clazz, withBlock, clazz.getMethodLocation().searchMethod("initialize")));
247249
clazz.addMethod(context, "__jallocate!", new StaticJCreateMethod(clazz, withBlock, null));
248250
} catch (SecurityException | NoSuchMethodException e) {
249251
// TODO log?
@@ -321,36 +323,38 @@ public static final class SplitCtorData {
321323
// public fields used by RealClassGenerator's generated code
322324
public final Object[] arguments;
323325
public final int ctorIndex;
324-
326+
325327
// public field used by finishInitialized & (ruby < ruby < java) generated classes
326328
public final IRubyObject[] rbarguments;
327329
public final Block block;
328330

329331
// fields below are only used in ConcreteJavaProxy finishInitialize
330332
private final AbstractIRMethod method;
333+
private final RubyModule clazz;
331334
private final String name;
332335
private final SplitSuperState<?> state;
336+
private final SplitCtorData nested;
333337

334338
/**
335339
* Picks and converts arguments for the super call
336340
* Leaves ctorIndex and arguments ready for the super call
337341
*/
338342
SplitCtorData(Ruby runtime, IRubyObject[] args, JCtorCache cache) {
339-
this(runtime, args, cache, null, null, null, Block.NULL_BLOCK);
343+
this(runtime, args, cache, null, null, null, null, Block.NULL_BLOCK, null);
340344
}
341345

342346
SplitCtorData(Ruby runtime, IRubyObject[] args, JCtorCache cache,
343-
AbstractIRMethod method, SplitSuperState<?> state, Block block) {
344-
this(runtime, args, cache, method, null, state, block);
347+
AbstractIRMethod method, RubyModule clazz, String name, Block block) {
348+
this(runtime, args, cache, method, clazz, name, null, block, null);
345349
}
346350

347351
SplitCtorData(Ruby runtime, IRubyObject[] args, JCtorCache cache,
348-
AbstractIRMethod method, String name, Block block) {
349-
this(runtime, args, cache, method, name, null, block);
352+
AbstractIRMethod method, RubyModule clazz, String name, SplitSuperState<?> state, Block block) {
353+
this(runtime, args, cache, method, clazz, name, state, block, null);
350354
}
351355

352356
private SplitCtorData(Ruby runtime, IRubyObject[] args, JCtorCache cache,
353-
AbstractIRMethod method, String name, SplitSuperState<?> state, Block block) {
357+
AbstractIRMethod method, RubyModule clazz, String name, SplitSuperState<?> state, Block block, SplitCtorData nested) {
354358
rbarguments = args;
355359
if (cache == null) { // (ruby < ruby < java) super call from one IRO to another IRO ctor
356360
ctorIndex = -1;
@@ -361,9 +365,24 @@ private SplitCtorData(Ruby runtime, IRubyObject[] args, JCtorCache cache,
361365
}
362366

363367
this.method = method;
368+
this.clazz = clazz;
369+
this.name = name;
370+
this.state = state;
371+
this.block = block;
372+
this.nested = nested;
373+
}
374+
375+
private SplitCtorData(SplitCtorData ctorData, IRubyObject[] args,
376+
AbstractIRMethod method, RubyModule clazz, String name, SplitSuperState<?> state, Block block) {
377+
this.arguments = ctorData.arguments;
378+
this.ctorIndex = ctorData.ctorIndex;
379+
this.rbarguments = args;
380+
this.method = method;
381+
this.clazz = clazz;
364382
this.name = name;
365383
this.state = state;
366384
this.block = block;
385+
this.nested = ctorData;
367386
}
368387
}
369388

@@ -376,30 +395,57 @@ public SplitCtorData splitInitialized(RubyClass base, IRubyObject[] args, Block
376395
final Ruby runtime = getRuntime();
377396
final String name = base.getClassConfig().javaCtorMethodName;
378397
final CacheEntry methodEntry = base.searchWithCache(name);
398+
final RubyClass sourceLocation = findIncludedPrependedModule(methodEntry.sourceModule, base);
399+
final RubyModule effectiveSource = sourceLocation != null ? sourceLocation : methodEntry.sourceModule;
379400
final boolean isLateral = isClassOrIncludedPrependedModule(methodEntry.sourceModule, base);
380401
DynamicMethod method = methodEntry.method.getRealMethod(); // ensure we don't use a wrapper (jruby/jruby#8148)
381402
if (method instanceof StaticJCreateMethod) method = ((StaticJCreateMethod) method).oldInit;
382403

404+
if (isPrependedJavaCtorWrapper(sourceLocation, base) && method instanceof AbstractIRMethod air) {
405+
SplitSuperState<?> state = air.startSplitSuperCall(runtime.getCurrentContext(), this, effectiveSource, name, args, block);
406+
IRubyObject[] forwardedArgs = state == null ? args : state.callArrayArgs.toJavaArrayMaybeUnsafe();
407+
Block forwardedBlock = state == null ? block : state.callBlockArgs;
408+
SplitCtorData ctorData = splitInitialized(sourceLocation.getSuperClass(), forwardedArgs, forwardedBlock, jcc);
409+
return new SplitCtorData(ctorData, args, air, effectiveSource, name, state, block);
410+
}
411+
383412
// jcreate is for nested ruby classes from a java class
384413
if (isLateral && method instanceof AbstractIRMethod) {
385414

386415
AbstractIRMethod air = (AbstractIRMethod) method; // TODO: getMetaClass() ? or base? (below v)
387416

388-
SplitSuperState<?> state = air.startSplitSuperCall(runtime.getCurrentContext(), this, getMetaClass(), name, args, block);
417+
SplitSuperState<?> state = air.startSplitSuperCall(runtime.getCurrentContext(), this, effectiveSource, name, args, block);
389418
if (state == null) { // no super in method
390-
return new SplitCtorData(runtime, args, jcc, air, name, block);
419+
return new SplitCtorData(runtime, args, jcc, air, effectiveSource, name, block);
391420
}
392-
return new SplitCtorData(runtime, state.callArrayArgs.toJavaArrayMaybeUnsafe(), jcc, air, state, block);
421+
return new SplitCtorData(runtime, state.callArrayArgs.toJavaArrayMaybeUnsafe(), jcc, air, effectiveSource, name, state, block);
393422
}
394423
return new SplitCtorData(runtime, args, jcc);
395424
}
396425

426+
private static boolean isPrependedJavaCtorWrapper(final RubyClass methodSource, final RubyClass klass) {
427+
if (methodSource == null) return false;
428+
429+
return methodSource != klass && methodSource.isIncluded() &&
430+
methodSource.getSuperClass() != null && methodSource.getSuperClass().getDelegate() == klass;
431+
}
432+
433+
private static RubyClass findIncludedPrependedModule(final RubyModule methodSource, final RubyClass klass) {
434+
RubyModule ceiling = klass.getMethodLocation();
435+
436+
for (RubyClass candidate = klass.getSuperClass(); candidate != null && candidate != ceiling; candidate = candidate.getSuperClass()) {
437+
if (candidate.getOrigin() == methodSource.getOrigin()) return candidate;
438+
}
439+
440+
return null;
441+
}
442+
397443
private static boolean isClassOrIncludedPrependedModule(final RubyModule methodSource, final RubyClass klass) {
398-
if (methodSource == klass) return true;
444+
if (methodSource == klass || methodSource.getDelegate() == klass) return true;
399445

400446
RubyClass candidate = klass.getSuperClass();
401447
while (candidate != null && (candidate.isIncluded() || candidate.isPrepended())) { // up till 'real' superclass
402-
if (candidate == klass) return true;
448+
if (candidate == methodSource || candidate.getDelegate() == methodSource.getDelegate()) return true;
403449
candidate = candidate.getSuperClass();
404450
}
405451

@@ -412,11 +458,13 @@ private static boolean isClassOrIncludedPrependedModule(final RubyModule methodS
412458
* <p>Note: invoked from generated byte-code</p>
413459
*/
414460
public void finishInitialize(SplitCtorData returned) {
461+
if (returned.nested != null) finishInitialize(returned.nested);
462+
415463
if (returned.method != null) {
416464
if (returned.state != null) {
417465
returned.method.finishSplitCall(returned.state);
418466
} else { // no super, direct call
419-
returned.method.call(getRuntime().getCurrentContext(), this, getMetaClass(),
467+
returned.method.call(getRuntime().getCurrentContext(), this, returned.clazz,
420468
returned.name, returned.rbarguments, returned.block);
421469
}
422470
}

core/src/test/java/org/jruby/javasupport/TestJava.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,42 @@ public void testOverrideNewOnConcreteJavaProxySubClassRegression() {
139139
assertTrue(runtime.evalScriptlet("FormatImpl.new").toJava(Object.class) instanceof SimpleDateFormat);
140140
}
141141

142+
@Test
143+
public void testPrependedInitializeRunsForRubyClassesRootedInJavaSuperclass() {
144+
String script =
145+
"class PrependJavaBaseInitC < java.util.ArrayList\n" +
146+
" def initialize(trace)\n" +
147+
" trace << :c\n" +
148+
" super()\n" +
149+
" end\n" +
150+
"end\n" +
151+
"class PrependJavaBaseInitD < PrependJavaBaseInitC\n" +
152+
" def initialize(trace)\n" +
153+
" trace << :d\n" +
154+
" super\n" +
155+
" end\n" +
156+
"end\n" +
157+
"class PrependJavaBaseInitE < PrependJavaBaseInitD\n" +
158+
" def initialize(trace)\n" +
159+
" trace << :e\n" +
160+
" super\n" +
161+
" end\n" +
162+
"end\n" +
163+
"module PrependJavaBaseInitM\n" +
164+
" def initialize(trace)\n" +
165+
" trace << :m\n" +
166+
" super\n" +
167+
" end\n" +
168+
"end\n" +
169+
"PrependJavaBaseInitE.prepend(PrependJavaBaseInitM)\n" +
170+
"trace = []\n" +
171+
"PrependJavaBaseInitE.new(trace)\n" +
172+
"trace\n";
173+
174+
final Ruby runtime = Ruby.newInstance();
175+
assertEquals("[:m, :e, :d, :c]", runtime.evalScriptlet(script).toString());
176+
}
177+
142178
@Test
143179
public void testHashMethodOnJavaProxy() {
144180
final Ruby runtime = Ruby.newInstance();

spec/java_integration/reify/become_java_spec.rb

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,42 @@ class TopRightOfTheJStack < MiddleOfTheJStack ;def size; super + 3; end ; end
120120
expect(TopRightOfTheJStack.new([:a, :b]).size).to eq (2+3)
121121
end
122122

123+
it "calls prepended and leaf initialize methods for Ruby classes rooted in a Java superclass" do
124+
class PrependJavaBaseInitC < java.util.ArrayList
125+
def initialize(trace)
126+
trace << :c
127+
super()
128+
end
129+
end
130+
131+
class PrependJavaBaseInitD < PrependJavaBaseInitC
132+
def initialize(trace)
133+
trace << :d
134+
super
135+
end
136+
end
137+
138+
class PrependJavaBaseInitE < PrependJavaBaseInitD
139+
def initialize(trace)
140+
trace << :e
141+
super
142+
end
143+
end
144+
145+
module PrependJavaBaseInitM
146+
def initialize(trace)
147+
trace << :m
148+
super
149+
end
150+
end
151+
152+
PrependJavaBaseInitE.prepend(PrependJavaBaseInitM)
153+
154+
trace = []
155+
PrependJavaBaseInitE.new(trace)
156+
expect(trace).to eq([:m, :e, :d, :c])
157+
end
158+
123159
it "supports auto reifying a class hierarchy when class gets redefined" do
124160
class ASubList < java.util.ArrayList
125161
attr_reader :args

0 commit comments

Comments
 (0)