[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

cameron314 added a comment. In D53412#1450182 , @clayborg wrote: > All of these are in SBProcess. No need to change everywhere else, I just see > a ton of duplicated code here in the SBProcess.cpp. If we contain those in a > small struct/class, then we

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

clayborg added a comment. In general any call that is doing anything through the public API needs to take the target API mutex. It is ok for quick accessors or other things to not do so, but anything that would need to keep the process in its current state, like asking a frame for a register

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

clayborg added a comment. Added Jim Ingham in case he wants to chime in here. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53412/new/ https://reviews.llvm.org/D53412 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

clayborg added a comment. All of these are in SBProcess. No need to change everywhere else, I just see a ton of duplicated code here in the SBProcess.cpp. If we contain those in a small struct/class, then we can easily make changes as needed without 50 diffs in this file each time. So no need

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

cameron314 added a comment. There's dozens of places that take the API mutex without taking the process mutex. Take `Kill` for example: It needs to take the API mutex, but cannot take the run lock since it will be taken by the private state thread. Another example is `HandleCommand`, which

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

clayborg added a comment. In D53412#1446252 , @cameron314 wrote: > @clayborg, I'm not sure how that would work. There's many places that lock > the process run lock without locking the target API mutex, and vice versa. Add an argument to the

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

cameron314 added a comment. @clayborg, I'm not sure how that would work. There's many places that lock the process run lock without locking the target API mutex, and vice versa. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53412/new/

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

clayborg added a comment. Or better yet, create a structure that everyone must use and have the locking exist in the class itself: struct ProcessLocker { std::lock_guard guard; Process::StopLocker stop_locker; void ProcessLocker(Process ) { ... } }; Repository:

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

cameron314 added a comment. Herald added a project: LLDB. Anyone? We still have this patch applied on our recently-rebased fork with no problems... Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53412/new/ https://reviews.llvm.org/D53412

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

cameron314 added a comment. Ping? Repository: rLLDB LLDB https://reviews.llvm.org/D53412 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

cameron314 added a comment. I suppose we could but there's a few places outside of SBProcess that also use the run lock and API mutex; personally, I prefer it to be explicit which mutex is being taken first. Repository: rLLDB LLDB https://reviews.llvm.org/D53412

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

clayborg added a comment. Since this is duplicated in so many spots, can we make a helper function to do this to ensure no one gets it wrong. We might be able to pass in the "guard" and "stop_locker" as reference variables and modify them in the helper function. Repository: rLLDB LLDB

[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected

cameron314 created this revision. cameron314 added reviewers: zturner, clayborg. Herald added a subscriber: lldb-commits. This patch changes the order that mutexes are acquired in SBProcess such that the target API mutex is now always acquired before the public run lock mutex is acquired. This