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

[Jonas Devlieghere via Phabricator via lldb-commits]

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:

  $ echo "script foo = 1" > test.lldb
  $ lldb ./a.out
  (lldb) b main
  (lldb) breakpoint command add -s python
      frame.GetThread().GetProcess().GetTarget().GetDebugger().HandleCommand("command source -s true ./test.lldb")
      DONE
  (lldb) run


The way the IO handlers are currently managed by the debugger is wrong. The implementation lacks proper synchronization between `RunIOHandler` and `ExecuteIOHandlers`. The latter is meant to be run by the "main thread", while the former is meant to be run synchronously, potentially from a different thread. Imagine a scenario where `RunIOHandler` is called from a different thread than `ExecuteIOHandlers`. Both functions manipulate the debugger's `IOHandlerStack`. Although the `push` and `pop` operations are synchronized, the logic to activate, deactivate and run IO handlers is not.

While investigating https://llvm.org/PR44352, I noticed some weird behavior in the `Editline` implementation. One of its members (`m_editor_status`) was modified from another thread. This happened because the main thread, while running `ExecuteIOHandlers` ended up execution the `IOHandlerEditline` created by the breakpoint callback thread. Even worse, due to the lack of synchronization within the IO handler implementation, both threads ended up executing the same IO handler.

Given the way the IO handlers work, I don't see the need to have execute them synchronously. When an IO handler is pushed, it will interrupt the current handler unless specified otherwise. One exception where being able to run a handler synchronously is the sourcing of the `.lldbinit` file in the home and current working directory. This takes place *before* `ExecuteIOHandlers` is started from `RunCommandInterpreter`, which means that in the new scheme these two IO handlers end up at the bottom of the stack. To work around this specific problem, I've added an option to run the IO handler synchronously if needed (i.e. `ExecuteIOHandlers` is not running yet). When that's the case, any other threads are prevented (blocked) from starting to execute the IO handlers. I don't think this workaround is necessary for any other handlers.

I've been starting at this for quite a bit and tried a few different approaches, but it's totally possible that I missed some. I'm more than open to suggestions for more elegant solutions!


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D72748

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72748.238166.patch
Type: text/x-patch
Size: 11462 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200115/1136c1f4/attachment.bin>