[Lldb-commits] [PATCH] D75004: Fix a race between lldb's packet timeout and killing the profile thread

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Jim Ingham said in email:

> I don’t understand your suggestion.  The point of this change was to get the profile loop to exit without waiting the full sleep time.  That time is generally pretty long (1 second or thereabouts) and so if you have the main debugserver thread wait on the profile thread’s timeout before replying to the detach packet, then lldb might have already timed out waiting for the packet reply by the time it does so.

Gotcha, then yes ignore my comments for alt way...

> You could also fix this by not doing the pthread_join but instead have the detaching finish independently and then let the profile thread exit after the fact, but that seems like asking for trouble.  Or you could reply to the detach right away, and then wait on the profile thread to exit as debugserver is going down.  But the current code is not at all set up to do that.
> 
> Anyway, this solution has the benefit of being exactly what you want to have happen - the profile thread stops its wait sleep immediately when told to exit.  And it relies on well tested mechanisms, and doesn’t seem terribly intrusive.

That is true.

> BTW, I thought about doing the time calculation outside the profile thread loop, but I couldn’t see anything that said you have to stop and restart the profile loop when you change the timeout.  So it didn’t seem safe to only calculate the timeout once.  However, if we want to require that you stop and start the profile thread when you change its wait interval it would be fine to do that.

Fair point. All objections removed then, as this is the most performant and bullet proof solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75004