> 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!).

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.

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]>
wrote:

>
>
> Am 17.11.2021 um 14:42 schrieb 'Jesse Glick' via Jenkins Developers <
> [email protected]>:
>
> On Wed, Nov 17, 2021 at 5:57 AM Ullrich Hafner <[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
>
> 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].
> 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].
> 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].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CAF73ShDfhD6C%2BQ4bV76605%3DAU5V1zZ%2B0zAx8Sis9DFxTQ3URVg%40mail.gmail.com.

Reply via email to