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

Make sure we transmit the actual debug port to next dev mode run #41043

Merged
merged 1 commit into from
Jun 7, 2024
Merged

Make sure we transmit the actual debug port to next dev mode run #41043

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
22 changes: 8 additions & 14 deletions core/deployment/src/main/java/io/quarkus/deployment/dev/QuarkusDevModeLauncher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,6 @@ public B debug(String debug) {
return (B) this;
}

@SuppressWarnings("unchecked")
public B debugPortOk(Boolean debugPortOk) {
QuarkusDevModeLauncher.this.debugPortOk = debugPortOk;
return (B) this;
}

@SuppressWarnings("unchecked")
public B suspend(String suspend) {
QuarkusDevModeLauncher.this.suspend = suspend;
Expand Down Expand Up @@ -303,10 +297,10 @@ public R build() throws Exception {

private List<String> args = new ArrayList<>(0);
private String debug;
private Boolean debugPortOk;
private String suspend;
private String debugHost = "localhost";
private String debugPort = "5005";
private String actualDebugPort;
private File projectDir;
private File buildDir;
private File outputDir;
Expand Down Expand Up @@ -390,12 +384,13 @@ protected void prepare() throws Exception {

if (debug != null && debug.equalsIgnoreCase("client")) {
args.add("-agentlib:jdwp=transport=dt_socket,address=" + debugHost + ":" + port + ",server=n,suspend=" + suspend);
actualDebugPort = String.valueOf(port);
} else if (debug == null || !debug.equalsIgnoreCase("false")) {
// if the debug port is used, we want to make an effort to pick another one
// if we can't find an open port, we don't fail the process launch, we just don't enable debugging
// Furthermore, we don't check this on restarts, as the previous process is still running
boolean warnAboutChange = false;
if (debugPortOk == null) {
if (actualDebugPort == null) {
int tries = 0;
while (true) {
boolean isPortUsed;
Expand All @@ -408,20 +403,19 @@ protected void prepare() throws Exception {
isPortUsed = false;
}
if (!isPortUsed) {
debugPortOk = true;
actualDebugPort = String.valueOf(port);
break;
}
if (++tries >= 5) {
debugPortOk = false;
break;
} else {
port = getRandomPort();
}
}
}
if (debugPortOk) {
if (actualDebugPort != null) {
if (warnAboutChange) {
warn("Changed debug port to " + port + " because of a port conflict");
warn("Changed debug port to " + actualDebugPort + " because of a port conflict");
}
args.add("-agentlib:jdwp=transport=dt_socket,address=" + debugHost + ":" + port + ",server=y,suspend="
+ suspend);
Expand Down Expand Up @@ -547,8 +541,8 @@ public List<String> args() {
return args;
}

public Boolean getDebugPortOk() {
return debugPortOk;
public String getActualDebugPort() {
return actualDebugPort;
}

protected abstract boolean isDebugEnabled();
Expand Down
17 changes: 10 additions & 7 deletions devtools/maven/src/main/java/io/quarkus/maven/DevMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,15 +478,19 @@ public void close() throws IOException {
}
if (!changed.isEmpty()) {
getLog().info("Changes detected to " + changed + ", restarting dev mode");

// stop the runner before we build the new one as the debug port being free
// is tested when building the runner
runner.stop();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to do this in Gradle as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Your guess is as good as mine :). I will try to have a look.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see the restart logic in Gradle but I'm not very familiar with how it works.

Maybe @glefloch or @aloubyansky know more.

But I wouldn't block this PR as the current Maven behavior is a bit annoying.


final DevModeRunner newRunner;
try {
bootstrapId = handleAutoCompile();
newRunner = new DevModeRunner(runner.launcher.getDebugPortOk(), bootstrapId);
newRunner = new DevModeRunner(runner.launcher.getActualDebugPort(), bootstrapId);
} catch (Exception e) {
getLog().info("Could not load changed pom.xml file, changes not applied", e);
continue;
}
runner.stop();
newRunner.run();
runner = newRunner;
}
Expand Down Expand Up @@ -1171,8 +1175,8 @@ private DevModeRunner(String bootstrapId) throws Exception {
launcher = newLauncher(null, bootstrapId);
}

private DevModeRunner(Boolean debugPortOk, String bootstrapId) throws Exception {
launcher = newLauncher(debugPortOk, bootstrapId);
private DevModeRunner(String actualDebugPort, String bootstrapId) throws Exception {
launcher = newLauncher(actualDebugPort, bootstrapId);
}

Collection<Path> pomFiles() {
Expand Down Expand Up @@ -1226,7 +1230,7 @@ void stop() throws InterruptedException {
}
}

private QuarkusDevModeLauncher newLauncher(Boolean debugPortOk, String bootstrapId) throws Exception {
private QuarkusDevModeLauncher newLauncher(String actualDebugPort, String bootstrapId) throws Exception {
String java = null;
// See if a toolchain is configured
if (toolchainManager != null) {
Expand All @@ -1244,8 +1248,7 @@ private QuarkusDevModeLauncher newLauncher(Boolean debugPortOk, String bootstrap
.suspend(suspend)
.debug(debug)
.debugHost(debugHost)
.debugPort(debugPort)
.debugPortOk(debugPortOk)
.debugPort(actualDebugPort)
.deleteDevJar(deleteDevJar);

setJvmArgs(builder);
Expand Down
Loading