On Thu, 28 Mar 2024 13:09:10 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> This fix makes `AsyncGetCallTrace` reentrant and async-signal-safe. >> Reentrancy is required in the cases when two or more profiling engines are >> running at the same time, e.g., when CPU and Wall clock profilers work >> together and therefore one profiler may interrupt another in the middle of >> getting a stack trace. >> >> Tested with async-profiler: >> >> java >> -agentpath:/path/to/libasyncProfiler.so=start,event=cpu,interval=1ms,wall=1ms,file=profile.jfr > > I have my doubts as to whether AGCT is actually re-entrant in a general > sense, but I can see that the `ThreadInAsgct` RAII object introduced a > reentrancy constraint that did not exist prior, and so removing it should not > make AGCT any less safe and should allow previous reentrancy cases to > continue to work as before. @dholmes-ora Thank you for the review. When looking at the AGCT code, nothing suspicious that could affect reentrancy caught my eye. Also, benchmarks (specJVM, Renaissance) now run fine with two profilers enabled. So yes, while I don't have 100% proof that AGCT is async-signal-safe, the fix definitely improves the situation. ------------- PR Comment: https://git.openjdk.org/jdk/pull/18504#issuecomment-2026255860
