[jira] [Commented] (TORQUE-372) Apache Torque 6 concurrency problem

    [ https://issues.apache.org/jira/browse/TORQUE-372?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17944678#comment-17944678 ] 

Max Philipp Wriedt commented on TORQUE-372:
-------------------------------------------

{code:java}
- if (criteria == null #if ($useSimpleMapping) || (criteria.getJoins().isEmpty() && criteria.getSelectColumns().size() - offset ==  ${basePeerClassName}.numColumns) #end) 
+ if (criteria == null)
{code}
Is the "useSimpleMapping" option also affecting the concurrency?
In a project of mine this still provides a slight improvement to the performance. I don't have to deal with concurrency at all though.

> Apache Torque 6 concurrency problem
> -----------------------------------
>
>                 Key: TORQUE-372
>                 URL: https://issues.apache.org/jira/browse/TORQUE-372
>             Project: Torque
>          Issue Type: Bug
>          Components: Templates
>    Affects Versions: 6.0
>            Reporter: Harald Nuding
>            Priority: Major
>             Fix For: 7.0
>
>         Attachments: recordMapperBase-1.vm
>
>
> This was provided as e-mail from Harald Nuding 4th of Feb 2025:
> {noformat}
> We’re working on a 3pp uplift of an old project based on Torque 5.1 towards the new version 6.0.
> The migration was painless so far, so thank you all for the effort you spend!  
> During regression testing we run into a concurrency problem.
> The problem happens when multiple treads parallel initialize the Recordmapper of the same Table:
> java.util.ConcurrentModificationException: null
>                  at java.base/java.util.ArrayList.sort(ArrayList.java:1723) ~[?:?]
>                  at org.apache.torque.om.mapper.MappingStrategy.finish(MappingStrategy.java:70) ~[torque-runtime-6.0.jar:6.0]
>                  at app.backend.torque.BaseZoneRecordMapper.processRow(BaseZoneRecordMapper.java:635) ~[classes/:?]
>                  at app.backend.torque.BaseZoneRecordMapper.processRow(BaseZoneRecordMapper.java:1) ~[classes/:?]
>                  at org.apache.torque.util.ResultsetSpliterator.tryAdvance(ResultsetSpliterator.java:120) ~[torque-runtime-6.0.jar:6.0]
>                  at java.base/java.util.Spliterator.forEachRemaining(Spliterator.java:332) ~[?:?]
>                  at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) ~[?:?]
>                  at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[?:?]
>                  at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921) ~[?:?]
>                  at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
>                  at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682) ~[?:?]
>                  at org.apache.torque.util.BasePeerImpl.doSelect(BasePeerImpl.java:1153) ~[torque-runtime-6.0.jar:6.0]
>                  at org.apache.torque.util.BasePeerImpl.doSelect(BasePeerImpl.java:836) ~[torque-runtime-6.0.jar:6.0]
> Drilling down the stack, we were able to identify the root cause at the (singleton) initialisation of the Table 
> specific 
> Base<table name>RecordMapper
> The template definition of recordMapperBase.vm om module torque-templates produces concurrency sensitive code, 
> especially the lazy initialization of the strategy member at line 172ff. 
> (introduced with TORQUE-364: init a new Strategy implementation) 
>    
> 111    ## TORQUE-364: Should this be cached per RecordMapper (Thread safety/Multi query safety?)
> 112    private MappingStrategy<${dbObjectClassName}> strategy;
> ...
> 172                if (useMappingStrategy) {
> 173                    initStrategy();
> 174                }
> ...
> 194                        if (useMappingStrategy) 
> 195                        {
> 196                            strategy.addColumn(nextOffset, 
> 197                                (res, inst) -> inst.${setter}( ${getter}(res, nextOffset)));
> 198                        } else
> {noformat}
> Harald proposes a solution and attached a fix fro recordeMapperBase.vm:
> {noformat}
> I like to propose a solution for this problem:
> 1)	The comment at line 111 was right 😊
> 2)	initStrategy() should return the created MappingStrategy.
> 3)	The MappingStrategy value must be used within the processRow( .. ) method
> 4)	A double init of the strategy member does not do any harm, as long the created MappingStrategy instance is 
> used within the processRow( .. ) method
> 5)	Synchronisation or volatile on this.strategy is not needed then
> 6)	The boolean member useMappingStrategy can be omitted 
> I’ve attached our the modified and tested recordMapperBase.vm template file.
> ...
> {noformat}
> I start to test it soon and then put it into the repo trunk with comment like TORQUE-ID contributed by .. 
> Any comments welcome :)



--
This message was sent by Atlassian Jira
(v8.20.10#820010)