On Thu, 15 Jan 2026 11:58:34 GMT, Joel Sikström <[email protected]> wrote:
> Hello, > > Right now we check the null-marker for the NULLABLE_ATOMIC_FLAT LayoutKind > twice, both before and after allocating a heap instance. The first check > returns nullptr early if the null-marker is set, as we don't have to allocate > an object on the heap if we are going to return nullptr anyways. The other > check is redundant, since if the null-marker is not set, the source object is > non-null, which means the destination (res) object is non-null as well after > copying, so the code inside it is unreachable. > > Testing: > * Oracle's tier1-4, hotspot_valhalla, jdk_valhalla > * I also added a `gurantee` for the removed code and ran through > hotspot_valhalla and jdk_valhalla locally. The second check is needed because the source value might have been mutated between the first null-marker check and the value copying, and we don't want to have heap allocated instances being equivalent to a 'null' value. Here's the scenario: - thread 1 starts to read a nullable flat value, it checks that the value is not marked as null, then proceeds to copy the value but is preempted - thread 2 updates the nullable flat value by writing 'null' to it, which means setting the null-marker to zero, but also resetting all the other fields - thread 1 resumes its read, allocate the heap instance, and copy the payload Now, the problem is that there's a heap allocated instance with a payload that has been reset, which means fields contents might not be valid for this class. We don't want to have to check the null-marker each time we access a heap buffered value, we want to preserve the invariant that all heap buffered values are valid values for the class. So, the null marker is checked a second time in the heap allocated instance, and if it is zeroed, meaning the payload written to the instance is equivalent to null, then the instance is discarded. ------------- PR Comment: https://git.openjdk.org/valhalla/pull/1914#issuecomment-3754996915
