> Am 17.11.2021 um 16:36 schrieb 'Devin Nusbaum' via Jenkins Developers > <[email protected]>: > > > As far as I understood the problem in JENKINS-67145 > > <https://issues.jenkins.io/browse/JENKINS-67145> my step is somehow > > serialized during the execution in lines 205-215. > > Kind of, but I was talking about the publishIssues step, not the > scanForIssues step. I think the scanForIssues steps in the parallel branches > have already completed when the problem manifests and can be ignored based on > the information in the Jira ticket. > > > Or can I view the code in a step as an atomic unit? > > You only need to worry about the serialized state of the Pipeline, and in > this case I think you just need to worry about local variables in the > Pipeline (e.g. AnnotatedReport objects returned by scanForIssues) and the > StepExecution object for currently-running steps (e.g. publishIssues). Local > variables in PublishIssuesStep.Execution.run and other Java code can be > ignored. Also, since you are using SynchronousNonBlockingStepExecution, > PublishIssuesStep.Execution is not resumable > <https://github.com/jenkinsci/workflow-step-api-plugin/blob/58b25c2420773a1a6686da4b87990926cae994e9/src/main/java/org/jenkinsci/plugins/workflow/steps/SynchronousNonBlockingStepExecution.java#L75> > and you may as well mark Execution.step as transient (but I don't think that > would fix the issue!). >
Ah ok, this is what I thought initially. Then I can continue using my approach here :-) I totally misunderstood the problem. > In your case, I think the main issue is with the AnnotatedReport objects that > are returned by scanForIssues that end up being local variables in the > Pipeline. You need to be careful about the ways in which you modify those > objects in the pubishIssues step, since local Groovy variables are serialized > as part of the Pipeline's state, and PublishIssuesStep.Execution.run is > running on a background thread. My original message describes the only way > that I noticed that publishIssues might modify one of those existing > AnnotatedReport objects (on a background thread separate from the CPS VM > thread that will serialize the Pipeline) that was returned by a previous > scanForIssues step. > > My best guess as to minimally fixing the problem is to introduce a copy > constructor for FileStatistics and use it here > <https://github.com/jenkinsci/forensics-api-plugin/blob/d0dc4ec448ca6c9a0d5ce8a6d39e238ba07e8c30/src/main/java/io/jenkins/plugins/forensics/miner/RepositoryStatistics.java#L242> > like this: > > statisticsMapping.merge(additionalStatistics.getFileName(), new > FileStatistics(additionalStatistics), this::merge); > > This way, if one of the FileStatistics from one of the other AnnotatedReport > instances returned by a scanIssues step has the same file name, > RepositoryStatistics.merge will only end up modifying the copied > FileStatistics rather than the original object. > This is the problem, thanks! I thought that I am creating a new object but actually I am doing not. I’m changing the existing ones, that is a design flaw that I did overlook. I think a simple solution is to create new objects as you already mentioned. Thanks! > A more robust change would be to make AnnotatedReport and all of its fields > (and their fields recursively) immutable like Jesse mentioned. > > On Wed, Nov 17, 2021 at 10:02 AM Ullrich Hafner <[email protected] > <mailto:[email protected]>> wrote: > > >> Am 17.11.2021 um 14:42 schrieb 'Jesse Glick' via Jenkins Developers >> <[email protected] <mailto:[email protected]>>: >> >> On Wed, Nov 17, 2021 at 5:57 AM Ullrich Hafner <[email protected] >> <mailto:[email protected]>> wrote: >> […] is stored in a CopyOnWriteArrayList. If your background thread >> serializes my instance during the loop it can happen that the loop variable >> is already set, but the list with the results is not. >> >> Hard to follow what you are saying without a code reference, but a >> `CopyOnWriteArrayList` is designed to be safe to save from another thread; >> that is its whole purpose. >> >> I need some lock around the whole block of variables >> >> Block of fields? Again hard to discuss without a concrete example, but in >> general if there is a block of data that should be replaced atomically, best >> to keep it all in its own object with `final` fields, and refer to that one >> object with a `volatile` field. >> > > Actually I do not have fields anymore in the step: > https://github.com/jenkinsci/warnings-ng-plugin/blob/089ad6c3aac292bc926503e644b892371f609e88/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/ScanForIssuesStep.java#L205-L215 > > <https://github.com/jenkinsci/warnings-ng-plugin/blob/089ad6c3aac292bc926503e644b892371f609e88/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/ScanForIssuesStep.java#L205-L215> > > As far as I understood the problem in JENKINS-67145 > <https://issues.jenkins.io/browse/JENKINS-67145> my step is somehow > serialized during the execution in lines 205-215. Or can I view the code in a > step as an atomic unit? > >> -- >> You received this message because you are subscribed to the Google Groups >> "Jenkins Developers" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected] >> <mailto:[email protected]>. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr0ouBxhg1arOFwosdrit-O9WdBeiiAbgSAA00%3DPnbBkPQ%40mail.gmail.com >> >> <https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr0ouBxhg1arOFwosdrit-O9WdBeiiAbgSAA00%3DPnbBkPQ%40mail.gmail.com?utm_medium=email&utm_source=footer>. > > > -- > You received this message because you are subscribed to the Google Groups > "Jenkins Developers" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected] > <mailto:[email protected]>. > To view this discussion on the web visit > https://groups.google.com/d/msgid/jenkinsci-dev/80549ED3-122A-49E6-9327-0B2D3A376EB9%40gmail.com > > <https://groups.google.com/d/msgid/jenkinsci-dev/80549ED3-122A-49E6-9327-0B2D3A376EB9%40gmail.com?utm_medium=email&utm_source=footer>. > > -- > You received this message because you are subscribed to the Google Groups > "Jenkins Developers" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected] > <mailto:[email protected]>. > To view this discussion on the web visit > https://groups.google.com/d/msgid/jenkinsci-dev/CAF73ShDfhD6C%2BQ4bV76605%3DAU5V1zZ%2B0zAx8Sis9DFxTQ3URVg%40mail.gmail.com > > <https://groups.google.com/d/msgid/jenkinsci-dev/CAF73ShDfhD6C%2BQ4bV76605%3DAU5V1zZ%2B0zAx8Sis9DFxTQ3URVg%40mail.gmail.com?utm_medium=email&utm_source=footer>. -- You received this message because you are subscribed to the Google Groups "Jenkins Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/5B6A9690-E59E-41D7-BF59-7C4B63441404%40gmail.com.
