HHH-14244 Force dirty mark on write to uninitialized lazy property #3578
Conversation
.../main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/InlineDirtyCheckingHandler.java
Outdated
Show resolved
Hide resolved
hibernate-core/src/test/java/org/hibernate/jpa/test/enhancement/TestDirtyCheckOnLazy.java
Show resolved
Hide resolved
|
Usually PR is targeting |
hibernate-core/src/test/java/org/hibernate/jpa/test/enhancement/TestDirtyCheckOnLazy.java
Show resolved
Hide resolved
.../main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/InlineDirtyCheckingHandler.java
Outdated
Show resolved
Hide resolved
.../main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/InlineDirtyCheckingHandler.java
Outdated
Show resolved
Hide resolved
I was primarly interested on 5.4, so... :-) |
well, targeting |
|
Many thanks @pludov , this seems a rather important fix, but I'm rusty on ASM. I need to set aside some proper review time next week. |
| methodVisitor.visitJumpInsn( Opcodes.GOTO, uninitializedLazy ); | ||
|
|
||
| methodVisitor.visitLabel( skipUninitializedCheckWithPersistentAttributeInterceptor ); | ||
| if ( implementationContext.getClassFileVersion().isAtLeast( ClassFileVersion.JAVA_V6 ) ) { |
There was a problem hiding this comment.
Could you comment about the class version requirement?
I'm also wondering if we should test this on pre-6 java class versions, but that's going to be complicated with our CI setup as the main codebase requires Java 8.
There was a problem hiding this comment.
Stackmap bytecodes have been introduced in JAVA6 : https://stackoverflow.com/questions/25109942/what-is-a-stack-map-frame.
I just followed already existing pattern here (there was already that kind of test near the end).
| MethodVisitor methodVisitor, | ||
| Context implementationContext, | ||
| MethodDescription instrumentedMethod) { | ||
|
|
There was a problem hiding this comment.
just wondering, but was there a specific reason to have preferred using an ASM visitor over using a ByteBuddy template? Could you add a comment about the decision, for sake of future maintainers?
There was a problem hiding this comment.
No particular reason. I've just been following the already existing pattern in that class.
5 participants
The generated field writer method is modified.
I added a check to detect an uninitialized lazy field and force marking it dirty.
But that check is only usefull when the new value is the default for the type (null/0/false...). For other values, the existing comparison is enough to mark the dirty. So, the new code is only triggered when received the default value, to avoid doing the check too frequently