Skip to content

Commit

Permalink
Enforce assertions in the native image so that we can remove volatile…
Browse files Browse the repository at this point in the history
… from some TerminalOutput fields
  • Loading branch information
ppalaga committed Nov 11, 2020
1 parent 45b8d6c commit 52f46a3
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 10 deletions.
1 change: 1 addition & 0 deletions client/pom.xml
Expand Up @@ -125,6 +125,7 @@
--allow-incomplete-classpath
-H:IncludeResources=org/jboss/fuse/mvnd/.*
-H:-ParseRuntimeOptions
-ea
</buildArgs>
</configuration>
</plugin>
Expand Down
Expand Up @@ -71,18 +71,25 @@ public class TerminalOutput implements ClientOutput {

/** A sink for sending messages back to the daemon */
private volatile Consumer<Message> daemonDispatch;
private volatile String name;
private volatile int totalProjects;

/*
* The following non-final fields are read/written from the main thread only.
* This is guaranteed as follows:
* * The read/write ops are only reachable from accept(Message) and accept(List<Message>)
* * Both of these methods are guarded with "main".equals(Thread.currentThread().getName()) assertion
* Therefore, these fields do not need to be volatile
*/
private String name;
private int totalProjects;
/** String format for formatting the number of projects done with padding based on {@link #totalProjects} */
private volatile String projectsDoneFomat;
private volatile int maxThreads;
private String projectsDoneFomat;
private int maxThreads;
/** String format for formatting the actual/hidden/max thread counts */
private volatile String threadsFormat;

private int linesPerProject = 0; // read/written only by the displayLoop
private int doneProjects = 0; // read/written only by the displayLoop
private String buildStatus; // read/written only by the displayLoop
private boolean displayDone = false; // read/written only by the displayLoop
private String threadsFormat;
private int linesPerProject = 0;
private int doneProjects = 0;
private String buildStatus;
private boolean displayDone = false;

/**
* {@link Project} is owned by the display loop thread and is accessed only from there. Therefore it does not need
Expand Down

4 comments on commit 52f46a3

@gnodet
Copy link
Contributor

@gnodet gnodet commented on 52f46a3 Nov 11, 2020

Choose a reason for hiding this comment

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

@ppalaga I've just hit a problem with the assertions enabled.
The read input loop does send message directly and causes an assertion error : https://github.com/mvndaemon/mvnd/blob/52f46a3e9037d1a2b5e8d636ebbe54f0eb9c6e68/common/src/main/java/org/jboss/fuse/mvnd/common/logging/TerminalOutput.java#L347-L349
You can easily reproduce it by hitting + during any build.

@gnodet
Copy link
Contributor

@gnodet gnodet commented on 52f46a3 Nov 11, 2020

Choose a reason for hiding this comment

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

Exception in thread "Thread-1" java.lang.AssertionError
	at org.jboss.fuse.mvnd.common.logging.TerminalOutput.accept(TerminalOutput.java:140)
	at org.jboss.fuse.mvnd.common.logging.TerminalOutput.readInputLoop(TerminalOutput.java:359)
	at java.lang.Thread.run(Thread.java:834)
	at com.oracle.svm.core.thread.JavaThreads.threadStartRoutine(JavaThreads.java:517)
        at com.oracle.svm.core.posix.thread.PosixJavaThreads.pthreadStartRoutnine(PosixJavaThreads.java:192)  

@ppalaga
Copy link
Contributor Author

Choose a reason for hiding this comment

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

then feel free to revert 52f46a3. Now we know that the vars can be read from threads other than main.

@gnodet
Copy link
Contributor

@gnodet gnodet commented on 52f46a3 Nov 11, 2020

Choose a reason for hiding this comment

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

See #205 for a fix

Please sign in to comment.