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

Eliminate mutable global state in AbstractLoggingSpy #301

Merged
merged 1 commit into from
Jan 6, 2021
Merged

Eliminate mutable global state in AbstractLoggingSpy #301

Show file tree
Hide file tree
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
40 changes: 26 additions & 14 deletions daemon/src/main/java/org/apache/maven/cli/DaemonMavenCli.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ public class DaemonMavenCli {

private final Map<String, ConfigurationProcessor> configurationProcessors;

/** Non-volatile, assuming that it is accessed only from the main thread */
private AbstractLoggingSpy loggingSpy = AbstractLoggingSpy.dummy();

public DaemonMavenCli() throws Exception {
slf4jLoggerFactory = LoggerFactory.getILoggerFactory();
slf4jLogger = slf4jLoggerFactory.getLogger(this.getClass().getName());
Expand All @@ -177,12 +180,18 @@ public DaemonMavenCli() throws Exception {
public int main(List<String> arguments,
String workingDirectory,
String projectDirectory,
Map<String, String> clientEnv) throws Exception {
CliRequest req = new CliRequest(null, null);
req.args = arguments.toArray(new String[0]);
req.workingDirectory = workingDirectory;
req.multiModuleProjectDirectory = new File(projectDirectory);
return doMain(req, clientEnv);
Map<String, String> clientEnv,
AbstractLoggingSpy loggingSpy) throws Exception {
this.loggingSpy = loggingSpy;
try {
CliRequest req = new CliRequest(null, null);
req.args = arguments.toArray(new String[0]);
req.workingDirectory = workingDirectory;
req.multiModuleProjectDirectory = new File(projectDirectory);
return doMain(req, clientEnv);
} finally {
this.loggingSpy = AbstractLoggingSpy.dummy();
}
}

public int doMain(CliRequest cliRequest, Map<String, String> clientEnv) throws Exception {
Expand Down Expand Up @@ -261,7 +270,7 @@ void cli(CliRequest cliRequest)
}
} catch (ParseException e) {
System.err.println("Unable to parse maven.config: " + e.getMessage());
AbstractLoggingSpy.instance().append(MvndHelpFormatter.displayHelp(cliManager));
loggingSpy.log(MvndHelpFormatter.displayHelp(cliManager));
throw e;
}

Expand All @@ -273,14 +282,14 @@ void cli(CliRequest cliRequest)
}
} catch (ParseException e) {
System.err.println("Unable to parse command line options: " + e.getMessage());
AbstractLoggingSpy.instance().append(MvndHelpFormatter.displayHelp(cliManager));
loggingSpy.log(MvndHelpFormatter.displayHelp(cliManager));
throw e;
}
}

private void help(CliRequest cliRequest) throws Exception {
if (cliRequest.commandLine.hasOption(CLIManager.HELP)) {
AbstractLoggingSpy.instance().append(MvndHelpFormatter.displayHelp(new CLIManager()));
loggingSpy.log(MvndHelpFormatter.displayHelp(new CLIManager()));
throw new ExitException(0);
}

Expand Down Expand Up @@ -378,7 +387,7 @@ void logging(CliRequest cliRequest) {

private void version(CliRequest cliRequest) throws ExitException {
if (cliRequest.debug || cliRequest.commandLine.hasOption(CLIManager.VERSION)) {
AbstractLoggingSpy.instance().append(CLIReportingUtils.showVersion());
loggingSpy.log(CLIReportingUtils.showVersion());
if (cliRequest.commandLine.hasOption(CLIManager.VERSION)) {
throw new ExitException(0);
}
Expand Down Expand Up @@ -534,7 +543,7 @@ protected void configure() {
final EventSpyDispatcher eventSpyDispatcher = container.lookup(EventSpyDispatcher.class);
configure(cliRequest, eventSpyDispatcher, configurationProcessors);
populateRequest(cliRequest, cliRequest.request, slf4jLogger, eventSpyDispatcher,
container.lookup(ModelProcessor.class), createTransferListener(cliRequest));
container.lookup(ModelProcessor.class), createTransferListener(cliRequest), loggingSpy);
executionRequestPopulator.populateDefaults(cliRequest.request);
BootstrapCoreExtensionManager resolver = container.lookup(BootstrapCoreExtensionManager.class);
return Collections
Expand Down Expand Up @@ -941,7 +950,7 @@ private Object getLocation(Source source, File defaultLocation) {

private void populateRequest(CliRequest cliRequest) {
populateRequest(cliRequest, cliRequest.request, slf4jLogger, eventSpyDispatcher, modelProcessor,
createTransferListener(cliRequest));
createTransferListener(cliRequest), loggingSpy);
}

private static void populateRequest(
Expand All @@ -950,7 +959,8 @@ private static void populateRequest(
Logger slf4jLogger,
EventSpyDispatcher eventSpyDispatcher,
ModelProcessor modelProcessor,
TransferListener transferListener) {
TransferListener transferListener,
AbstractLoggingSpy loggingSpy) {
CommandLine commandLine = cliRequest.commandLine;
String workingDirectory = cliRequest.workingDirectory;
boolean showErrors = cliRequest.showErrors;
Expand Down Expand Up @@ -1051,7 +1061,9 @@ private static void populateRequest(

ExecutionListener executionListener = new ExecutionEventLogger();
if (eventSpyDispatcher != null) {
executionListener = new LoggingExecutionListener(eventSpyDispatcher.chainListener(executionListener));
executionListener = new LoggingExecutionListener(
eventSpyDispatcher.chainListener(executionListener),
loggingSpy);
}

String alternatePomFile = null;
Expand Down
84 changes: 7 additions & 77 deletions daemon/src/main/java/org/mvndaemon/mvnd/daemon/Server.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@
import java.util.function.Predicate;
import java.util.stream.Collectors;
import org.apache.maven.cli.DaemonMavenCli;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.project.MavenProject;
import org.mvndaemon.mvnd.builder.DependencyGraph;
import org.mvndaemon.mvnd.builder.SmartBuilder;
import org.mvndaemon.mvnd.common.DaemonConnection;
import org.mvndaemon.mvnd.common.DaemonException;
Expand All @@ -54,9 +51,7 @@
import org.mvndaemon.mvnd.common.DaemonStopEvent;
import org.mvndaemon.mvnd.common.Environment;
import org.mvndaemon.mvnd.common.Message;
import org.mvndaemon.mvnd.common.Message.BuildException;
import org.mvndaemon.mvnd.common.Message.BuildRequest;
import org.mvndaemon.mvnd.common.Message.BuildStarted;
import org.mvndaemon.mvnd.common.Os;
import org.mvndaemon.mvnd.daemon.DaemonExpiration.DaemonExpirationResult;
import org.mvndaemon.mvnd.daemon.DaemonExpiration.DaemonExpirationStrategy;
Expand Down Expand Up @@ -429,16 +424,14 @@ private void cancelNow() {

private void handle(DaemonConnection connection, BuildRequest buildRequest) {
updateState(Busy);
try {
final BlockingQueue<Message> sendQueue = new PriorityBlockingQueue<>(64,
Comparator.comparingInt(this::getClassOrder).thenComparingLong(Message::timestamp));
final BlockingQueue<Message> recvQueue = new LinkedBlockingDeque<>();
final AbstractLoggingSpy loggingSpy = new AbstractLoggingSpy(sendQueue);
try (ProjectBuildLogAppender logAppender = new ProjectBuildLogAppender(loggingSpy)) {

LOGGER.info("Executing request");

BlockingQueue<Message> sendQueue = new PriorityBlockingQueue<>(64,
Comparator.comparingInt(this::getClassOrder).thenComparingLong(Message::timestamp));
BlockingQueue<Message> recvQueue = new LinkedBlockingDeque<>();

DaemonLoggingSpy loggingSpy = new DaemonLoggingSpy(sendQueue);
AbstractLoggingSpy.instance(loggingSpy);
Thread sender = new Thread(() -> {
try {
boolean flushed = true;
Expand Down Expand Up @@ -538,7 +531,8 @@ public <T extends Message> T request(Message request, Class<T> responseType, Pre
buildRequest.getArgs(),
buildRequest.getWorkingDir(),
buildRequest.getProjectDir(),
buildRequest.getEnv());
buildRequest.getEnv(),
loggingSpy);
LOGGER.info("Build finished, finishing message dispatch");
loggingSpy.finish(exitCode);
} catch (Throwable t) {
Expand Down Expand Up @@ -633,68 +627,4 @@ public long getLastBusy() {
public String toString() {
return info.toString();
}

private static class DaemonLoggingSpy extends AbstractLoggingSpy {
private final BlockingQueue<Message> queue;

public DaemonLoggingSpy(BlockingQueue<Message> queue) {
this.queue = queue;
}

@Override
public void finish(int exitCode) throws Exception {
queue.add(new Message.BuildFinished(exitCode));
queue.add(Message.STOP_SINGLETON);
}

@Override
public void fail(Throwable t) throws Exception {
queue.add(new BuildException(t));
queue.add(Message.STOP_SINGLETON);
}

@Override
protected void onStartSession(MavenSession session) {
final int degreeOfConcurrency = session.getRequest().getDegreeOfConcurrency();
final DependencyGraph<MavenProject> dependencyGraph = DependencyGraph.fromMaven(session);
session.getRequest().getData().put(DependencyGraph.class.getName(), dependencyGraph);

final int maxThreads = degreeOfConcurrency == 1 ? 1 : dependencyGraph.computeMaxWidth(degreeOfConcurrency, 1000);
queue.add(new BuildStarted(getCurrentProject(session).getName(), session.getProjects().size(), maxThreads));
}

private MavenProject getCurrentProject(MavenSession mavenSession) {
// Workaround for https://issues.apache.org/jira/browse/MNG-6979
// MavenSession.getCurrentProject() does not return the correct value in some cases
String executionRootDirectory = mavenSession.getExecutionRootDirectory();
if (executionRootDirectory == null) {
return mavenSession.getCurrentProject();
}
return mavenSession.getProjects().stream()
.filter(p -> (p.getFile() != null && executionRootDirectory.equals(p.getFile().getParent())))
.findFirst()
.orElse(mavenSession.getCurrentProject());
}

@Override
protected void onStartProject(String projectId, String display) {
queue.add(Message.projectStarted(projectId, display));
}

@Override
protected void onStopProject(String projectId, String display) {
queue.add(Message.projectStopped(projectId, display));
}

@Override
protected void onStartMojo(String projectId, String display) {
queue.add(Message.mojoStarted(projectId, display));
}

@Override
protected void onProjectLog(String projectId, String message) {
queue.add(projectId == null ? Message.log(message) : Message.log(projectId, message));
}

}
}
110 changes: 59 additions & 51 deletions daemon/src/main/java/org/mvndaemon/mvnd/logging/smart/AbstractLoggingSpy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,83 +15,91 @@
*/
package org.mvndaemon.mvnd.logging.smart;

import java.util.Collection;
import java.util.function.Consumer;
import org.apache.maven.execution.ExecutionEvent;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.project.MavenProject;
import org.mvndaemon.mvnd.builder.DependencyGraph;
import org.mvndaemon.mvnd.common.Message;
import org.mvndaemon.mvnd.common.Message.BuildException;
import org.mvndaemon.mvnd.common.Message.BuildStarted;

/**
* Sends events back to the client.
*/
public class AbstractLoggingSpy {
private static final AbstractLoggingSpy DUMMY = new AbstractLoggingSpy(m -> {
});
private final Consumer<Message> queue;

public abstract class AbstractLoggingSpy {

private static AbstractLoggingSpy instance;

public static AbstractLoggingSpy instance() {
if (instance == null) {
instance = new AbstractLoggingSpy() {
};
}
return instance;
}

public static void instance(AbstractLoggingSpy instance) {
AbstractLoggingSpy.instance = instance;
}

public void append(String event) {
append(null, event);
}

public void append(String projectId, String event) {
String msg = event.endsWith("\n") ? event.substring(0, event.length() - 1) : event;
onProjectLog(projectId, msg);
}

public void finish(int exitCode) throws Exception {
}

public void fail(Throwable t) throws Exception {
}

protected void notifySessionStart(ExecutionEvent event) {
onStartSession(event.getSession());
/**
* @return a dummy {@link AbstractLoggingSpy} that just swallows the messages and does not send them anywhere
*/
public static AbstractLoggingSpy dummy() {
return DUMMY;
}

protected void notifySessionFinish(ExecutionEvent event) {
onFinishSession();
public AbstractLoggingSpy(Collection<Message> queue) {
this(queue::add);
}

protected void notifyProjectBuildStart(ExecutionEvent event) {
onStartProject(getProjectId(event), getProjectDisplay(event));
public AbstractLoggingSpy(Consumer<Message> queue) {
this.queue = queue;
}

protected void notifyProjectBuildFinish(ExecutionEvent event) {
onStopProject(getProjectId(event), getProjectDisplay(event));
}
public void sessionStarted(ExecutionEvent event) {
final MavenSession session = event.getSession();
final int degreeOfConcurrency = session.getRequest().getDegreeOfConcurrency();
final DependencyGraph<MavenProject> dependencyGraph = DependencyGraph.fromMaven(session);
session.getRequest().getData().put(DependencyGraph.class.getName(), dependencyGraph);

protected void notifyMojoExecutionStart(ExecutionEvent event) {
onStartMojo(getProjectId(event), getProjectDisplay(event));
final int maxThreads = degreeOfConcurrency == 1 ? 1 : dependencyGraph.computeMaxWidth(degreeOfConcurrency, 1000);
queue.accept(new BuildStarted(getCurrentProject(session).getName(), session.getProjects().size(), maxThreads));
}

protected void notifyMojoExecutionFinish(ExecutionEvent event) {
onStopMojo(getProjectId(event), getProjectDisplay(event));
public void projectStarted(ExecutionEvent event) {
queue.accept(Message.projectStarted(getProjectId(event), getProjectDisplay(event)));
}

protected void onStartSession(MavenSession session) {
public void projectLogMessage(String projectId, String event) {
String msg = event.endsWith("\n") ? event.substring(0, event.length() - 1) : event;
queue.accept(projectId == null ? Message.log(msg) : Message.log(projectId, msg));
}

protected void onFinishSession() {
public void projectFinished(ExecutionEvent event) {
queue.accept(Message.projectStopped(getProjectId(event), getProjectDisplay(event)));
}

protected void onStartProject(String projectId, String display) {
public void mojoStarted(ExecutionEvent event) {
queue.accept(Message.mojoStarted(getProjectId(event), getProjectDisplay(event)));
}

protected void onStopProject(String projectId, String display) {
public void finish(int exitCode) throws Exception {
queue.accept(new Message.BuildFinished(exitCode));
queue.accept(Message.STOP_SINGLETON);
}

protected void onStartMojo(String projectId, String display) {
public void fail(Throwable t) throws Exception {
queue.accept(new BuildException(t));
queue.accept(Message.STOP_SINGLETON);
}

protected void onStopMojo(String projectId, String display) {
public void log(String msg) {
queue.accept(Message.log(msg));
}

protected void onProjectLog(String projectId, String message) {
private MavenProject getCurrentProject(MavenSession mavenSession) {
// Workaround for https://issues.apache.org/jira/browse/MNG-6979
// MavenSession.getCurrentProject() does not return the correct value in some cases
String executionRootDirectory = mavenSession.getExecutionRootDirectory();
if (executionRootDirectory == null) {
return mavenSession.getCurrentProject();
}
return mavenSession.getProjects().stream()
.filter(p -> (p.getFile() != null && executionRootDirectory.equals(p.getFile().getParent())))
.findFirst()
.orElse(mavenSession.getCurrentProject());
}

private String getProjectId(ExecutionEvent event) {
Expand Down
Loading