[PATCH] D76073: [compiler-rt][tsan] Fix: Leak of ThreadSignalContext memory mapping when destroying fibers.

Sat Apr 11 01:34:29 PDT 2020

dvyukov added a comment.

In D76073#1969708 <https://reviews.llvm.org/D76073#1969708>, @Florian wrote:

> I integrated your feedback and put a separate callback for just the clean up. To not do any further refactoring I placed it in OnFinished where also other resources of ThreadState are freed.
>  The cleanup accesses non of the previously freed resources and does not free any resources needed in the destructor called after it or the stat aggregation, which are the only functions accessing ThreadState afterwards.
>
> PS: It is my very first time contributing to a bigger repository/project and I have only limited experience with big codebases. I really try to get this right and not cause any issues, maybe I was a bit over-eager with my refactoring. Thank you for all your patience and feedback :)

Thanks! It's now way more obvious what is the problem, what is the fix and what are effects of this change.
Refactorings are good, but sometimes they better go in separately for purposes of review/description/history/testing/reverts/etc.
Thanks for your persistence!

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76073/new/

https://reviews.llvm.org/D76073