wolfpld / tracy

Frame profiler
https://tracy.nereid.pl/
Other
8.67k stars 598 forks source link

TRACY_MANUAL_LIFETIME crash with TracyLockable #689

Closed plekakis closed 6 months ago

plekakis commented 6 months ago

Hello,

I am using TRACY_MANUAL_LIFETIME and TRACY_DELAYED_INIT so only a specific process of my toolchain allows tracing. Now, the CPU & GPU markets, I can workaround that by using the TracyVkNamedZoneC and ZoneNamedNC macros and passing the initialised flag as the "enabled" argument. This initialised flag is updated at my main program's initialisation. As a result this will be fine, however mutex instrumentation relies on the tracy context to be valid at construction time, so it will currently crash if tracy is initialised delayed.

Is there plan to add support for checking if the srcloc is valid and skip the mutex instrumentation if not? tracy_force_inline SharedLockableCtx( const SourceLocationData* srcloc )

Or perhaps any other solution that will prevent such crashes if tracy is initialised manually.

Best regards

plekakis commented 6 months ago

On a sidenote, I can see that most tracy functions would crash if TRACY_MANUAL_LIFETIME is used. Is this simply something that hasn't been tested/actively developed?

wolfpld commented 6 months ago

Manual lifetime assumes users would perform their own tracking when Tracy can and can't be used.

plekakis commented 6 months ago

This is fine with most functions and macros apart from the mutex LockableCtx though, as that relies on tracy having been initialised previously. I can already handle the cases where tracy can be used for CPU & GPU profiling, it's only mutexes that pose a problem currently.

wolfpld commented 6 months ago

Yes, it's possible that there would be difficulties in some cases. I don't think the original author of this particular functionality required working lockables support, and I don't use the manual lifetime code paths either. If you can work this out, I can accept a PR.

plekakis commented 6 months ago

Ok, I will try to fix this today, I think that I know what needs to be done.

plekakis commented 6 months ago

Ok, PR submitted. I tested this locally against both MANUAL_LIFETIME defined and not, behaviour is correct and I don't see any problems. Maybe this is a viable fix.