On Tue, 2 Apr 2024 11:16:55 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Calling extra logging calls during initialization of Logger libraries can >> cause recursion leading to StackOverflowError >> This patch adds logic to the EventHelper class to detect recursion and >> prevent it. > > You are right, turns out the jtr file did have the StackOverFlowError from > the test introduced in this PR - I just didn't check it given the other > issue. > > Now that I look at the stacktrace and the complex checks we are adding, I am > starting to wonder if these re-entrancy checks should actually be added > within the `jdk.internal.logger` implementation code/classes, instead of at > these call sites. As far as I can see, the `jdk.internal.event.EventHelper` > isn't doing anything out of the ordinary and the same issue could probably > end up in some other parts during the logging initialization. @jaikiran I did look at adding logic in the Logger libraries also to correct this issue. Significant work was done in that area last year via JDK-8314263 I guess the question is what sort of Logger and LoggerFinder would you return in cases where recursion is detected ? Extra issues might arise from the System.getLogger method being public and widely used. Additional issues might stem from the fact that Logger libraries can be plugged in and returned Logger might be stored as a static variable etc. It seemed safer to catch the issue at the EventHelper level - a library that's confined to internal JDK use. ------------- PR Comment: https://git.openjdk.org/jdk/pull/18534#issuecomment-2032295584
