Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-68071] Java 17 compat: make CpsThreadGroup.paused transient to avoid XStreaming AtomicBoolean #515

Merged
merged 1 commit into from
Mar 18, 2022
Merged

[JENKINS-68071] Java 17 compat: make CpsThreadGroup.paused transient to avoid XStreaming AtomicBoolean #515

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThreadGroup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,12 @@ public final class CpsThreadGroup implements Serializable {
* on hold (for example while inspecting a suspicious state or to perform a maintenance
* when a failure is predictable.)
*/
private /*almost final*/ AtomicBoolean paused = new AtomicBoolean();
private /*almost final*/ transient AtomicBoolean paused = new AtomicBoolean();

/**
* Persistent version of {@link #paused}.
*/
private boolean executionPaused;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might not hurt to make this volatile? No strong preference.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe make it Boolean and only set it to a non-null value in writeReplace (and null it out in readResolve).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for volatile since it is not read except during deserialization.

Considered setting it during writeReplace but seemed like overkill since it is only ever written in two places and it just as easy to set it then.


/**
* "Exported" closures that are referenced by live {@link CpsStepContext}s.
Expand Down Expand Up @@ -183,15 +188,15 @@ private Object readResolve() {
script.setBinding(shell.getContext());
}
}
if (paused == null) { // introduced and later removed from serial form
paused = new AtomicBoolean(executionPaused);
}
return this;
}

private void setupTransients() {
runner = new CpsVmExecutorService(this);
pausedByQuietMode = new AtomicBoolean();
if (paused == null) { // earlier versions did not have this field.
paused = new AtomicBoolean();
}
}

@CpsVmThreadOnly
Expand Down Expand Up @@ -355,6 +360,7 @@ public void run() {
*/
public Future<?> pause() {
paused.set(true);
executionPaused = true;
// CPS VM might have a long queue in its task list, so to properly ensure
// that the execution has actually suspended, call scheduleRun() excessively
return scheduleRun();
Expand All @@ -365,6 +371,7 @@ public Future<?> pause() {
*/
public void unpause() {
if (paused.getAndSet(false)) {
executionPaused = false;
// some threads might have became executable while we were pausing.
scheduleRun();
} else {
Expand Down