nrknithin commented on code in PR #6686:
URL:
https://github.com/apache/incubator-kie-drools/pull/6686#discussion_r3240057430
##########
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:
This race is pre-existing — the original setMode had no synchronization at
all. This PR added synchronized to the write path to fix a sequential
state-pollution case JUnit 6 ordering exposed: TraitTest (parameterized
MAP/TRIPLES tests) left the static factory in TRIPLES mode, then
TraitTypeGenerationTest ran in MAP mode and got the wrong proxy constructor
(the "konst is null" failure). The fix re-syncs the factory to the instance's
mode at the top of buildProxyClass, which solves the sequential case. Fully
fixing the unsynchronized reads (the concurrent case you flagged) needs a
redesign — per-instance builders or full-method locking — out of scope for the
SB 4 upgrade.
@yesamer @gitgabrio do u think we need to create a follow up issue for this?
--
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]
