[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

clayborg added a comment. In D72748#1830638 , @labath wrote: > In D72748#1829956 , @JDevlieghere > wrote: > > > In D72748#1823945 , @labath wrote: > > > > > I didn't

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

labath added a comment. In D72748#1829956 , @JDevlieghere wrote: > In D72748#1823945 , @labath wrote: > > > I didn't actually try it but I am pretty sure this will deadlock with > > nested lldb command files

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

This revision was automatically updated to reflect the committed changes. Closed by commit rG04de24e690d3: [lldb/IOHandler] Improve synchronization between IO handlers. (authored by JDevlieghere). Changed prior to commit: https://reviews.llvm.org/D72748?vs=238886=239178#toc Repository: rG

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

JDevlieghere marked an inline comment as done. JDevlieghere added a comment. In D72748#1823945 , @labath wrote: > I didn't actually try it but I am pretty sure this will deadlock with nested > lldb command files (running `command source` from a file that

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Let's give this a shot. I'm don't think this is fully right, but I also don't know how to improve that, exactly... Comment at:

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

JDevlieghere updated this revision to Diff 238886. JDevlieghere added a comment. Add PExpect test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72748/new/ https://reviews.llvm.org/D72748 Files: lldb/include/lldb/Core/Debugger.h

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

labath added a comment. Not super ideal, but not too bad either. Not clicking accept yet because of the missing test case... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72748/new/ https://reviews.llvm.org/D72748 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

JDevlieghere updated this revision to Diff 238684. JDevlieghere added a comment. Use recursive mutex CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72748/new/ https://reviews.llvm.org/D72748 Files: lldb/include/lldb/Core/Debugger.h lldb/source/Core/Debugger.cpp Index:

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

labath added a comment. I didn't actually try it but I am pretty sure this will deadlock with nested lldb command files (running `command source` from a file that is itself being sourced). Changing the mutex to a recursive_mutex would fix that, but I don't believe it would make this fully

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

JDevlieghere updated this revision to Diff 238358. JDevlieghere added a comment. Synchronize between `RunIOHandler` and `ExecuteIOHandlers` using a mutex. When running an IO handler synchronously, block `ExecuteIOHandlers` until we're finished. CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

JDevlieghere planned changes to this revision. JDevlieghere added a comment. I'm working on a different approach that should address al the concerns raised so far. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72748/new/ https://reviews.llvm.org/D72748

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

jingham added a comment. In D72748#1822443 , @clayborg wrote: > So I did a bunch of original IOHandler. And there are many complex cases for > sure. One thing to be aware of is that if you won't use editline() and we > call fgets() in the default

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

clayborg added a comment. So I did a bunch of original IOHandler. And there are many complex cases for sure. One thing to be aware of is that if you won't use editline() and we call fgets() in the default implementation, there is no way to cancel this IIRC. So it might be worth trying this

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

labath added a comment. Though I have messed with IOHandlers in the past, I have successfully suppressed most of the memories of it. I think I have a rough understanding of what the bug is, but I don't understand the solution yet. With this patch, what does guarantee that the IOHandler for the

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

JDevlieghere updated this revision to Diff 238172. JDevlieghere added a comment. Rebase on top of NFC change CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72748/new/ https://reviews.llvm.org/D72748 Files: lldb/include/lldb/Core/Debugger.h lldb/source/Core/Debugger.cpp

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

JDevlieghere added a comment. I didn't get a chance to write a test case yet, but you can reproduce the problem with the following setup: $ echo "script foo = 1" > test.lldb $ lldb ./a.out (lldb) b main (lldb) breakpoint command add -s python

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

JDevlieghere created this revision. JDevlieghere added reviewers: labath, jingham, friss. Herald added a subscriber: abidh. Herald added a project: LLDB. JDevlieghere added a comment. I didn't get a chance to write a test case yet, but you can reproduce the problem with the following setup: $