Copilot commented on code in PR #6686:
URL:
https://github.com/apache/incubator-kie-drools/pull/6686#discussion_r3239882319
##########
drools-traits/src/main/java/org/drools/traits/core/factmodel/AbstractTraitFactory.java:
##########
@@ -194,6 +198,10 @@ protected static String getKey(Class core, Class trait) {
}
protected Class<T> buildProxyClass(K core, Class<?> trait) {
+ // The trait class builder factory is static and may have been left in
a different
+ // VirtualPropertyMode by an earlier call. Re-sync it to this
factory's mode so the
+ // generated proxy class has constructors that match the lookup in
cacheConstructor.
+ syncBuilderFactoryWithMode(mode);
Review Comment:
`syncBuilderFactoryWithMode(mode)` only synchronizes the switch that mutates
the static `traitClassBuilderFactory`, but `buildProxyClass` then performs
wrapper + proxy generation after releasing the class-level lock. If another
thread calls `setMode(...)` (or `buildProxyClass` with a different mode)
concurrently, the factory builders can change mid-build, leading to a
wrapper/proxy generated with mixed modes (and the constructor mismatch this
change is trying to avoid). Consider synchronizing the entire proxy/wrapper
generation section (including builder lookups) on the same lock, or removing
the mutable static factory state (e.g., keep per-mode builder instances and
select without mutation).
##########
build-parent/pom.xml:
##########
@@ -121,15 +123,15 @@
<version.jakarta.json-api>2.1.3</version.jakarta.json-api>
<version.org.apache.openjpa>4.0.0</version.org.apache.openjpa>
<version.org.jpmml.model>1.6.4</version.org.jpmml.model> <!-- jpmml-model
BSD 3C license - ATTENTION 1.5.1 intentional, because 1.5.1 evaluators works
with 1.5.1 -->
- <version.org.junit.jupiter>5.13.4</version.org.junit.jupiter>
- <version.org.junit.platform>1.13.4</version.org.junit.platform> <!-- Keep
synchronized with junit-jupiter (middle and minor should be the same) -->
+ <!-- JUnit 6: single pin for jupiter + platform + vintage via junit-bom.
See version.junit above for the legacy JUnit 4 API. -->
Review Comment:
The new comment says the JUnit 6 version is pinned "via junit-bom", but this
POM is not actually importing `org.junit:junit-bom` in `dependencyManagement`
(versions are set directly on each JUnit artifact). Please either import the
BOM (preferred if the intent is BOM-based alignment) or adjust the comment to
reflect the actual mechanism used here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
