Remove unnecessary synchronization in attach API logging #2827
Conversation
pdbain-ibm
force-pushed
the
attach_logging
branch
from
84b0998 to
345e545
Compare
| File logFile = new File(logName + pidProperty + ".log"); //$NON-NLS-1$ | ||
| IPC.logStream = new PrintStream(logFile); | ||
| IPC.setDefaultVmId(pidProperty); | ||
| IPC.logMessage("AttachHandler initialize"); //$NON-NLS-1$ |
There was a problem hiding this comment.
This message won't be logged since logging is not yet enabled.
There was a problem hiding this comment.
Right. Changing to unconditional print.
There was a problem hiding this comment.
Or you can set LOGGING_ENABLED before calling logMessage()
| } | ||
| } | ||
|
|
||
| private static void printMessageWithHeader(String msg, PrintStream log) { |
There was a problem hiding this comment.
This method should have a comment that it must be called from inside a synchronized block.
| IPC.logStream = new PrintStream(logFile); | ||
| IPC.setDefaultVmId(pidProperty); | ||
| IPC.logMessage("AttachHandler initialize"); //$NON-NLS-1$ | ||
| loggingStatus = LOGGING_ENABLED; |
There was a problem hiding this comment.
I don't think this implementation is 100% proper. In theory the IPC logging code could see that logging is enabled before the logstream is fully initialized.
There was a problem hiding this comment.
logStream is initialized and used inside synchronized blocks, so it cannot be used until initialization is complete.
Avoid synchronization when the enabled state of logging is known. Signed-off-by: Peter Bain <peter_bain@ca.ibm.com>
pdbain-ibm
force-pushed
the
attach_logging
branch
from
345e545 to
e67da56
Compare
|
jenkins test sanity plinux jdk8 |
Avoid synchronization when the enabled state of logging is known.
volatiledeclaration of the logging status variable.loggingStatusis in a terminal state (enabled or disabled), avoid synchronizing.loggingStatusfor the most probable value directly without a method call.Fixes #2048
@pshipton would you kindly review this? Thank you.
Signed-off-by: Peter Bain peter_bain@ca.ibm.com