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.

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

Pick random debug port when the configured one is taken #33548

Merged
merged 1 commit into from
Oct 16, 2023
Merged
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -369,26 +369,50 @@ protected void prepare() throws Exception {
port = Integer.parseInt(debug);
}
}
int originalPort = port;
if (port <= 0) {
port = getRandomPort();
}

if (debug != null && debug.equalsIgnoreCase("client")) {
args.add("-agentlib:jdwp=transport=dt_socket,address=" + debugHost + ":" + port + ",server=n,suspend=" + suspend);
} else if (debug == null || !debug.equalsIgnoreCase("false")) {
// make sure the debug port is not used, we don't want to just fail if something else is using it
// we don't check this on restarts, as the previous process is still running
// if the debug port is used, we want to make an effort to pick another one
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't we print a info/warn that is locating a free port because specified one is not working?

btw. do we really need the 5 retries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point about the error message - added.

btw. do we really need the 5 retries?

It's just a detail honestly...

Copy link
Member

Choose a reason for hiding this comment

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

just tested - and yes; we really need to print something about it not using the expected configured port.

// 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) {
try (Socket socket = new Socket(getInetAddress(debugHost), port)) {
error("Port " + port + " in use, not starting in debug mode");
debugPortOk = false;
} catch (IOException e) {
debugPortOk = true;
int tries = 0;
while (true) {
boolean isPortUsed;
try (Socket socket = new Socket(getInetAddress(debugHost), port)) {
// we can make a connection, that means the port is in use
isPortUsed = true;
warnAboutChange = warnAboutChange || (originalPort != 0); // we only want to warn if the user had not configured a random port
} catch (IOException e) {
// no connection made, so the port is not in use
isPortUsed = false;
}
if (!isPortUsed) {
debugPortOk = true;
break;
}
if (++tries >= 5) {
debugPortOk = false;
break;
} else {
port = getRandomPort();
}
}
}
if (debugPortOk) {
if (warnAboutChange) {
warn("Changed debug port to " + port + " because of a port conflict");
}
args.add("-agentlib:jdwp=transport=dt_socket,address=" + debugHost + ":" + port + ",server=y,suspend="
+ suspend);
} else {
error("Port " + port + " in use, not starting in debug mode");
}
}

Expand Down