RFR: 8367949: JFR: MethodTrace double-counts methods that catch their own exceptions

[Robert Toyonaga]

On Sun, 21 Dec 2025 16:22:25 GMT, Erik Gahlin <egahlin at openjdk.org> wrote:

> Could I have a review of a PR that changes how the instrumentation of the MethodTrace and MethodTiming events is implemented, so they handle exceptions in a better way? 
> 
> For constructors, the current implementation is still used in certain corner cases. A proper implementation would require data-flow analysis, but for all practical purposes this code should work fine.
> 
> Testing: jdk/jdk/jfr
> 
> Thanks
> Erik

src/jdk.jfr/share/classes/jdk/jfr/internal/tracing/Transform.java line 176:

> 174:         }
> 175:         TryBlock last = tryBlocks.getLast();
> 176:         if (tryBlocks.getLast().end == null) {

Suggestion:

        if (last.end == null) {

Is it important to read `tryBlocks.getLast()` again here?

test/jdk/jdk/jfr/event/tracing/TestConstructors.java line 116:

> 114:             }
> 115:             try {
> 116:                 new Zebra(true);

This results in `Zebra(int)` getting traced but not `Zebra(boolean)` because an exception is thrown and the  `Zebra(boolean)` constructor call [is outside the `try` block](https://github.com/openjdk/jdk/pull/28947/files#diff-68a37600bc91d54808ea1ca427ade6af8a600889877f262e20782c550eded410R160). Is this intended?  Shouldn't a method be traced every time it is called?  In contrast, `new Zebra(false);` causes both `Zebra(int)` and `Zebra(boolean)` to be traced.

Additionally, with the old approach, `new Cat();` would not cause `Cat()` to be traced at all, since its callee, `methodThatThrows()`, prevents execution ever reaching `Cat()`'s `return` statement. I did a quick check on this by hardcoding `simplifiedInstrumentation = true`. Now, with the new approach in this PR,  `new Cat();` causes `Cat()` to be traced exactly once. This makes sense to me, but is different than before.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/28947#discussion_r2666613894
PR Review Comment: https://git.openjdk.org/jdk/pull/28947#discussion_r2666618915