On Tue, 17 Oct 2023 19:54:24 GMT, Johannes Bechberger <jbechber...@openjdk.org>
wrote:
>> Fix `onthrow` issue by passing the event info to the `initialize` method.
>>
>> This prevents `jdb` from receiving a broken exception event and throwing an
>> internal NullPointerException, upon attaching to the JDWP-agent.
>
> Johannes Bechberger has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Add suggested modification
src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c line 631:
> 629: * Initialize debugger back end modules
> 630: *
> 631: * @param opt_info optional event info to use, might be null
No java code here, so Javadoc syntax isn't appropriate. I suggest to remove the
`@param` notation.
test/jdk/com/sun/jdi/JdwpOnThrowTest.java line 3:
> 1: /*
> 2: * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights
> reserved.
> 3: * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
Change to SAP copyright
`* Copyright (c) 2023 SAP SE. All rights reserved.`
test/jdk/com/sun/jdi/JdwpOnThrowTest.java line 29:
> 27: import com.sun.jdi.connect.Connector;
> 28: import com.sun.jdi.connect.IllegalConnectorArgumentsException;
> 29: import com.sun.jdi.connect.ListeningConnector;
Unneeded imports:
import com.sun.jdi.connect.ListeningConnector;
import com.sun.jdi.request.EventRequestManager;
import jdk.test.lib.Utils;
import java.net.Inet4Address;
import java.net.Inet6Address;
import java.net.InetAddress;
test/jdk/com/sun/jdi/JdwpOnThrowTest.java line 119:
> 117: }
> 118:
> 119: private static String ATTACH_CONNECTOR = "com.sun.jdi.SocketAttach";
move the `private static` fields up to the other place where `private static
long TIMEOUT = 10000;` is declared to have them together.
test/jdk/com/sun/jdi/JdwpOnThrowTest.java line 145:
> 143: return connector;
> 144: }
> 145: }
Suggestion:
for (Connector connector :
Bootstrap.virtualMachineManager().allConnectors()) {
if (connector.name().equalsIgnoreCase(name)) {
return connector;
}
}
test/jdk/com/sun/jdi/ThrowCaughtException.java line 3:
> 1: /*
> 2: * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights
> reserved.
> 3: * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
Change to SAP copyright
`* Copyright (c) 2023 SAP SE. All rights reserved.`
test/jdk/com/sun/jdi/ThrowCaughtException.java line 36:
> 34:
> 35: class Ex extends RuntimeException {
> 36: }
Add a line break at the end.
test/jdk/com/sun/jdi/lib/jdb/Debuggee.java line 71:
> 69: private String address = null;
> 70: private boolean suspended = true;
> 71: private String onthrow = "";
Debugee.java needs a copyright year update.
test/jdk/com/sun/jdi/lib/jdb/Debuggee.java line 195:
> 193:
> 194: String getTransport() {
> 195: if (address == null) {
Is the address check appropriate/required here (in the getTransport() method)?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16145#discussion_r1364499380
PR Review Comment: https://git.openjdk.org/jdk/pull/16145#discussion_r1364499780
PR Review Comment: https://git.openjdk.org/jdk/pull/16145#discussion_r1364533293
PR Review Comment: https://git.openjdk.org/jdk/pull/16145#discussion_r1364542094
PR Review Comment: https://git.openjdk.org/jdk/pull/16145#discussion_r1364538431
PR Review Comment: https://git.openjdk.org/jdk/pull/16145#discussion_r1364501944
PR Review Comment: https://git.openjdk.org/jdk/pull/16145#discussion_r1364532446
PR Review Comment: https://git.openjdk.org/jdk/pull/16145#discussion_r1364532810
PR Review Comment: https://git.openjdk.org/jdk/pull/16145#discussion_r1364503907
