Skip to content

Commit

Permalink
#646 Add volatile where required for double checked locking
Browse files Browse the repository at this point in the history
Without volatile we can have the situation where thread 2 sees the value of initPending to be false and thus doesn't go through the synced section but sees loggers as not yet visible.

This was rare and the test only failed on the first run on my machine before always passing.

Hopefully, it should always work now. Fingers crossed.

NOTE: Both of the changed classes use a version of the "double checked locking" pattern which the internet says requires volatile.
  • Loading branch information
danfickle committed Feb 11, 2021
1 parent 2de727f commit e06e641
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@
* https://github.com/danfickle/openhtmltopdf/wiki/Logging
*/
public class JDKXRLogger implements XRLogger {
private boolean initPending = true;
private volatile boolean initPending = true;

// Keep a map of Loggers so they are not garbage collected
// which makes them lose their settings we have applied.
private Map<String, Logger> loggers;
private volatile Map<String, Logger> loggers;

private final boolean useParent;
private final Level level;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ private static String registerLoggerByName(final String loggerName) {
return loggerName;
}

private static boolean initPending = true;
private static XRLogger loggerImpl;
private static volatile boolean initPending = true;
private static volatile XRLogger loggerImpl;

private static volatile Boolean loggingEnabled;

Expand Down

0 comments on commit e06e641

Please sign in to comment.