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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to