On Fri, 8 May 2026 19:06:35 GMT, Serguei Spitsyn <[email protected]> wrote:
>> I'll let Serguei clarify, but here's what I found. The logic to decide if a >> snapshot (clone) is needed can be found in this diff (from the PR that >> implemented this support): >> >> https://github.com/openjdk/valhalla/pull/2363/changes#diff-41730062a596d5a0b6b2330022428347b22d71d1a654af76e9959be7a75f65fcR642 >> >> My read on this is that if you are in a constructor, accessing local 0 >> (this), and the current frame's method is for a value class, then the >> snapshot will be done. That would mean when executing in the Number >> constructor, this snapshot behavior would apply even if a BigInteger is >> being constructed. This seems to be consistent with what is done at the >> language level (not allowing "this" to escape from the Number constructor, >> even when constructing a BigInteger). >> >> Now my question is, what about when executing in the BigInteger constructor? >> Even though it is not a value type, it appears java does not allow escape of >> "this", presumably because it extends a value type. If we are going to do >> the same in JVMTI, we would need to do some extra checks of the inheritance, >> but I don't think there is actually any harm in JVMTI exposing "this" of >> BigInteger before the Number constructor is executed since it is actually an >> identity object. > >> this snapshot behavior would apply even if a BigInteger is being constructed. > > My understanding that it should not. There is a check for `obj->is_inline()`. > >> but I don't think there is actually any harm in JVMTI exposing "this" of >> BigInteger before the Number constructor is executed since it is actually an >> identity object. > > Agreed. I see three tests in that code: - `_jvf->method()->is_object_constructor()`: we're looking at an `<init>` method (I _think_ that's all this means?) - `_index == 0`: trying to limit the behavior to `this`, but local 0 might not be `this`, and a different local might be `this` - `obj != nullptr && obj->is_inline()`: the value of the local is a value object (again, guessing at what this code does) If I'm reading right, there's not test for a _value class_, but there is a test that the object is a _value object_. So that's the description I offered above: > When preview features are enabled, if the requested local is the "this" > variable > of a constructor, and it references a value object, the value_ptr is set to a > snapshot of the > value object. I'm concerned about "the this variable of a constructor", though, because that's not really what the code is doing. To more accurately specify it, it should be "if the 0th local variable of a constructor is requested". At that point we've got a spec and implementation in alignment. Great! But the the goal of the behavior has not been achieved, which is to defend against observable mutation of a larval value object (i.e., an `uninitializedThis` local variable that is a value object). I don't think you have any reliable way to tell which local variables might be `this`, and so to be safe we've got to cast a wider net and clone _all_ value objects accessed through a value class constructor. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2412#discussion_r3210794405
