Release ref count on timing information when freeing draw state.

[litghost]

No description provided.

[kmurray]

@litghost Could you add a comment describing why this is needed?

I think its because the draw state will otherwise hold onto the timing analyzer forever (shared_ptr semantics). But this shouldn't cause any real memory leaks in the standard tool flow, since whenever the graphics were passed a new timing analyzer its assigned to draw_sate->setup_timing_info, which would decrement the ref count on the old (previously held analyzer, and increment the new ref count). Correct?

[litghost]

@litghost Could you add a comment describing why this is needed?

Sure

I think its because the draw state will otherwise hold onto the timing analyzer forever (shared_ptr semantics). But this shouldn't cause any real memory leaks in the standard tool flow, since whenever the graphics were passed a new timing analyzer its assigned to draw_sate->setup_timing_info, which would decrement the ref count on the old (previously held analyzer, and increment the new ref count). Correct?

Your analysis doesn't account for what happens VPR is terminating. draw_sate->setup_timing_info is never cleared, then it's refcount is always 1 at termination. As a result it looks like a leak of the setup_timing_info at termination.

[litghost]

It is interesting though, the shared_ptr should be cleared after main exits, but asan/leak sanitizer did find a leak related to this shared_ptr. Adding this reset resolved the leak sanitizer report, but I wonder if it might be an error in leak sanitizer?

[kmurray]

It is interesting though, the shared_ptr should be cleared after main exits, but asan/leak sanitizer did find a leak related to this shared_ptr.

Yes, I'd expect it to be cleaned-up then as well.

Just speculating, but I've seen cases in the past in VPR, where constructors/destructors weren't being called correctly since the containing struct was allocated with malloc instead of new (motivating #506). Perhaps that could be the case with draw_state?

[litghost]

Just speculating

Good idea, but nope: https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/master/vpr/src/draw/draw_global.cpp#L22 . I'll need to dig into this later.