[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

[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

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. 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. @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/ https://reviews.llvm.org/

[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 take

[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 c

[Lldb-commits] [PATCH] D89236: [lldb] Fix bitfield "frame var" for pointers (pr47743)

cameron314 added a comment. This fixes https://bugs.llvm.org/show_bug.cgi?id=47743 for me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89236/new/ https://reviews.llvm.org/D89236 ___ lldb-commits mailin

[Lldb-commits] [PATCH] D68641: [LLDB] Fix for synthetic children memory leak

cameron314 created this revision. cameron314 added reviewers: labath, jingham. cameron314 added a project: LLDB. Herald added subscribers: lldb-commits, JDevlieghere, aprantl. This fixes a memory leak with several synthetic children front-ends. The lifetime of a ValueObject and all its derivative

[Lldb-commits] [PATCH] D68641: [LLDB] Fix for synthetic children memory leak

cameron314 added a comment. @aprantl No. A weak pointer would still fix the memory leak, but it's safe to use a raw pointer because we only reference objects which are in the same cluster as the synthetic children front-end itself. The other (leak-free) synthetic front-ends do this as well. We

[Lldb-commits] [PATCH] D68641: [LLDB] Fix for synthetic children memory leak

cameron314 added a comment. Right, I'll add comments on the pointer declarations. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68641/new/ https://reviews.llvm.org/D68641 ___ lldb-commits mailing list lldb-commits@

[Lldb-commits] [PATCH] D68641: [LLDB] Fix for synthetic children memory leak

cameron314 updated this revision to Diff 223919. cameron314 added a comment. Added comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68641/new/ https://reviews.llvm.org/D68641 Files: source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp source/Plugins/Language/CPlusPlus/LibCxxOp

[Lldb-commits] [PATCH] D68641: [LLDB] Fix for synthetic children memory leak

cameron314 added a comment. Any objections before I commit? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68641/new/ https://reviews.llvm.org/D68641 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[Lldb-commits] [PATCH] D68641: [LLDB] Fix for synthetic children memory leak

This revision was automatically updated to reflect the committed changes. Closed by commit rG89386daa9571: [LLDB] Fix for synthetic children memory leak (authored by cameron314). Changed prior to commit: https://reviews.llvm.org/D68641?vs=223919&id=224112#toc Repository: rG LLVM Github Monor

[Lldb-commits] [PATCH] D68641: [LLDB] Fix for synthetic children memory leak

cameron314 added a comment. I've committed a fix in rG745e57c5939e . Sorry about that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68641/new/ https://reviews.llvm.org/D68641

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp created this revision. tetsuo-cpp added a reviewer: xbolva00. tetsuo-cpp added projects: LLVM, LLDB. Herald added subscribers: lldb-commits, JDevlieghere. Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=39405 alexc@kitty:~/work/wiredtiger/build_posix$ cat breakpoint.json [{"Breakpo

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp updated this revision to Diff 81. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68179/new/ https://reviews.llvm.org/D68179 Files: lldb/include/lldb/Utility/JSON.h lldb/source/Utility/JSON.cpp lldb/source/Utility/StructuredData.cpp lldb/tools/debugserver/source/JSON

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp added a comment. In D68179#1690073 , @JDevlieghere wrote: > Hey, I just want to give you a heads up that I'm in the process to replace > LLDB's JSON implementation with the one from LLVM. The parts in > StructuredData are already gone (r37335

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp updated this revision to Diff 222923. tetsuo-cpp added a comment. Rebased onto trunk. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68179/new/ https://reviews.llvm.org/D68179 Files: lldb/tools/debugserver/source/JSON.cpp lldb/tools/debugserver/source/JSON.h Index: lldb

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp added a comment. Thank you for the suggestions! I'm assuming that since only the `debugserver` portion of this change is left, I should write a test specifically for that. I had a quick look and it wasn't obvious how to do that but I will spend some more time on it this weekend. @ji

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp updated this revision to Diff 228561. Herald added a subscriber: mgorny. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68179/new/ https://reviews.llvm.org/D68179 Files: lldb/tools/debugserver/source/JSON.cpp lldb/tools/debugserver/source/JSON.h lldb/unittests/debugserver

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp added a comment. I've had a try at writing some unit tests for the JSON parser in `debugserver`, including the empty array case which I'm fixing in this patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68179/new/ https://reviews.llvm.org/D68179 ___

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp added a comment. Friendly ping. Could I please get this looked at? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68179/new/ https://reviews.llvm.org/D68179 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.l

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp updated this revision to Diff 229291. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68179/new/ https://reviews.llvm.org/D68179 Files: lldb/tools/debugserver/source/JSON.cpp lldb/tools/debugserver/source/JSON.h lldb/unittests/debugserver/CMakeLists.txt lldb/unittests/de

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp marked an inline comment as done. tetsuo-cpp added a comment. Thanks for looking at this. I will need someone to commit it for me. However, I've been having issues with the test suite on my MacBook. `check-lldb-unit` works for me but `check-llvm` and `check-lldb` are hitting issues be

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp added a comment. Thanks for the reviews! @davide could you please commit this on my behalf? Thanks for the advice. I intend to continue working on LLDB when I can so I will spend some time to fix the test suite on my Mac. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68179/n