Skip to content

Commit

Permalink
fix: Daemon invoker lifespan must be shared with daemon process lifes…
Browse files Browse the repository at this point in the history
…pan (#1196)

* fix: Daemon lifecycle must be shared with daemon invoker

As long daemon is alive, the daemon invoker must be kept
alive as it holds the "resident" object graph of Maven.

* Add hack

This is a bug in resident invoker
  • Loading branch information
cstamas authored Nov 8, 2024
1 parent 2b3937b commit 7726c16
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 24 deletions.
2 changes: 1 addition & 1 deletion daemon/src/main/java/org/apache/maven/cli/DaemonCli.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
/**
* Simple interface to bridge maven 3.9.x and 4.0.x CLI
*/
public interface DaemonCli {
public interface DaemonCli extends AutoCloseable {
int main(
List<String> args,
String workingDir,
Expand Down
52 changes: 34 additions & 18 deletions daemon/src/main/java/org/apache/maven/cli/DaemonMavenCling.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,30 @@
import org.codehaus.plexus.classworlds.realm.ClassRealm;
import org.mvndaemon.mvnd.cli.EnvHelper;

/**
* The main Daemon entry point: it shares lifecycle with daemon invoker (subclass of resident invoker) that keeps Maven
* resident, as long as this class is used. Once not needed, proper shut down using {@link #close()} method
* is required.
* <p>
* While daemon invoker is stateful (keeps Maven object graph), daemon parser is stateless and reusable, no need to
* create instance per incoming call.
*/
public class DaemonMavenCling implements DaemonCli {
private final DaemonMavenParser parser;
private final DaemonMavenInvoker invoker;

public DaemonMavenCling() {
this.parser = new DaemonMavenParser();
this.invoker = new DaemonMavenInvoker(ProtoLookup.builder()
.addMapping(
ClassWorld.class, ((ClassRealm) Thread.currentThread().getContextClassLoader()).getWorld())
.build());
}

@Override
public void close() {
invoker.close();
}

@Override
public int main(
Expand All @@ -49,24 +72,17 @@ public int main(
EnvHelper.environment(workingDir, env);
System.setProperty("maven.multiModuleProjectDirectory", projectDir);

try (DaemonMavenInvoker invoker = new DaemonMavenInvoker(ProtoLookup.builder()
.addMapping(
ClassWorld.class, ((ClassRealm) Thread.currentThread().getContextClassLoader()).getWorld())
.addMapping(BuildEventListener.class, buildEventListener)
.build())) {
DaemonMavenParser parser = new DaemonMavenParser();
return invoker.invoke(parser.parse(ParserRequest.builder(
"mvnd", "Maven Daemon", args, new ProtoLogger(), new DaemonMessageBuilderFactory())
.cwd(Paths.get(workingDir))
.in(in)
.out(out)
.err(err)
.lookup(ProtoLookup.builder()
.addMapping(Environment.class, () -> env)
.addMapping(BuildEventListener.class, buildEventListener)
.build())
.build()));
}
return invoker.invoke(parser.parse(ParserRequest.builder(
"mvnd", "Maven Daemon", args, new ProtoLogger(), new DaemonMessageBuilderFactory())
.cwd(Paths.get(workingDir))
.in(in)
.out(out)
.err(err)
.lookup(ProtoLookup.builder()
.addMapping(Environment.class, () -> env)
.addMapping(BuildEventListener.class, buildEventListener)
.build())
.build()));
}

/**
Expand Down
18 changes: 14 additions & 4 deletions daemon/src/main/java/org/apache/maven/cli/DaemonMavenInvoker.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,19 @@ public DaemonMavenInvoker(ProtoLookup protoLookup) {
super(protoLookup);
}

@Override
protected void configureLogging(LocalContext context) throws Exception {
super.configureLogging(context);
// TODO: this is a hack, and fixes issue in DefaultResidentMavenInvoker that does not copy TCCL
private ClassLoader tccl;

protected int doInvoke(LocalContext context) throws Exception {
try {
if (tccl != null) {
context.currentThreadContextClassLoader = tccl;
Thread.currentThread().setContextClassLoader(context.currentThreadContextClassLoader);
}
return super.doInvoke(context);
} finally {
this.tccl = context.currentThreadContextClassLoader;
}
}

@Override
Expand Down Expand Up @@ -85,7 +95,7 @@ private PrintStream printStream(OutputStream outputStream) {

@Override
protected org.apache.maven.logging.BuildEventListener doDetermineBuildEventListener(LocalContext context) {
return protoLookup.lookup(BuildEventListener.class);
return context.invokerRequest.lookup().lookup(BuildEventListener.class);
}

@Override
Expand Down
6 changes: 5 additions & 1 deletion daemon/src/main/java/org/mvndaemon/mvnd/daemon/Server.java
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,11 @@ public void close() {
try {
registry.close();
} finally {
socket.close();
try {
socket.close();
} finally {
cli.close();
}
}
}
}
Expand Down

0 comments on commit 7726c16

Please sign in to comment.