[lldb-dev] Locking issues

[Malea, Daniel]

Thanks for the clarifications about the Python code. Glad it's not on the
critical path and that LLDB can still be built without python support.


>Yeah, I explored this option when we first did the read/write lock and as
>soon as you have more than one primitive object (mutex, condition,
>read/write lock,
>etc), you are asking for even more trouble.

I think condition variable + lock are meant to work together.

>> 
>> 
>> The natural benefit of the above approach is that all the
>> locking/unlocking magic happen inside Process, and SB has to have 0
>> knowledge about Process locks.
>
>So how would you solve the problem where you want to play with items in
>the process and keep the process from going anywhere? Like
>
>SBProcess process = ...
>SBThread thread;
>uint32_t num_threads = process.GetNumThreads();
>for (uint32_t i=0; i<num_threads; ++i)
>{
>	thread = process.GetThreadAtIndex(i);
>}
>
>In the above code we let process do all the locking we could end up
>having the process change our from underneath us as we iterate?
>
>Another example:
>
>SBFrame frame;
>uint32_t num_frames = thread.GetNumFrames()
>for (...)
>{
>    frame = thread.GetFrameAtIndex(i);
>}


What's the context the above code is running in? I see two possibilities:

If there's a thread that are running the above, as well as another thread
that calls say SBProcess.Resume(), but neither of these threads are
invoked from within LLDB, I don't think there's any implicit locking that
can prevent process from changing. If implicit-locks are needed, the best
thing to do there, IMHO, is to invalidate the stale thread objects (when
the Process resumes) and have them error-out when used.. I'm not sure how
two r/w locks (or external locking) can help that use case as any locks
are held in the SB layer. How does the code above signal that it's done
with the process locks? If this use-case needs to be handled in a
non-surprising fashion, I think the only thing to do is to lift lock
acquisition one step above the SB layer and put the locks in the users
hands via an explicit SBProcess.Lock/Unlock() or some such mechanism.
Implicit locking as part of the SB functions doesn't seem like it would be
deadlock friendly..

On the other hand, if the loops above are running in a 'callback' context.
That is, invoked from within LLDB as part of some event notification
mechanism, I foresee LLDB spawning a thread with the above code right
before a condition_variable::wait(), thereby 'promising' to keep process
unchanged in the meantime. The same promise has to be kept by the code
above; so reading threads is fine, but evaluating expressions (that are
more complicated than reading some memory) or calling
process.Step/Resume() should cause errors. Anything the code above wants
to do to modify process state would have to be queued somewhere and
processed after control is passed back to the debugger.


>> 
>>>> Is it not possible to have both "driving" code and "callback"
>>>> code go through exactly the same code path the SB* layer as well as
>>>>the
>>>> Process class?
>>> 
>>> You mean using the same locks? I am confused by your question. All code
>>> uses the same public API through the SB* layer. The SB layer locks you
>>> out when it is dangerous to do things while the process is running.
>>>Many
>>> of the issues we ran into stemmed from Xcode accessing the SB* API from
>>> multiple threads and all of the locks (the target lock and the process
>>> read/write (stopped/running) locks are needed.
>> 
>> Sorry I was not too clear -- basically I'm worried about the complexity
>>of
>> having two discrete R/W locks here, and separate behaviour based on
>> internal/external threads.
>
>As opposed to taking a condition variable, locking it, and doing two of
>three other things? I am not clear how the new scheme simplifies things
>yet.

I think a condition variable + a lock is simpler than two r/w locks. I may
be missing something though; I'll try to create a patch that implements
what I'm envisioning here and I'll let you know how it goes. It's possible
that what I want is not possible :)

>> Since processes have one state, it "feels" like
>> there should be one lock.
>> 
>> 
>>> Let me know what you think after reading the above explanation. The
>>>locks
>>> are needed and necessary and we haven't been able to come up with
>>>another
>>> way to control access to the things we need to do from the various
>>> threads. We currently have two clients:
>>> - public clients
>>> - private process state thread clients
>>> 
>>> public clients get locked out, as they should, when the process is
>>> running (continue, or stepping) so we don't get useless answers.
>>> 
>>> Do you really want to evaluate the value for "x" when we are in the
>>> middle of a single step? No you want to get an error stating "error:
>>> process is running".
>> 
>> Right, expression evaluation would be one of those "non-const" cases
>>that
>> should error out if Process m_in_transition is true.
>
>I really don't understand the new scheme you are suggesting. What does
>m_in_transition mean? In transition between what and what?

The name is not ideal..it could use more work. Basically, the condition
variable/bool is just a flag that gets set when certain functions (that
modify the inferior process state) become forbidden. I see it being set
true when the callback code is executed (I.e. Through
OperatingSystemPython). In such contexts, any code that wants to modify
the inferior state should not be allowed because the inferior is already
in the process of being modified by whatever called into
OperatingSystemPython. Previously I called these functions 'const' but I
see now that is not correct. What I really mean is read-only with respect
to public and private state.

>Feel free to make a patch with your suggested changes and send it out and
>we can try it out on our stuff over here and maybe this will help us
>evaluate it better.
>
>You might then try to compile your new LLDB.framework on MacOSX and then
>run Xcode with it:
>
>DYLD_FRAMEWORK_PATH=/path/to/build/Debug
>/Applications/Xcode.app/Contents/MacOS/Xcode
>
>This will run Xcode with your new LLDB and allow you to test it out to
>make sure it all works.

Thanks! Are there any simple unit tests though that use LLDB and don't
depend on Xcode? Maybe I can help write some.. What kind of
callback-registration mechanisms should I be looking at? I'm only familiar
with SBBreakpoint.SetCallback() which I imagine is the same thing as is
exercised by the TestStopHookMultipleThreads.py suite?

Does anything exist yet to register functions to be called on process
state change?

I'll try to implement what I proposed (maybe with the simplification of
the private run lock to a simple mutex) and let you know if it fails
miserably or is something you should look at.

Anyways, I'm probably not going to get to this before next week, so any
hints or thoughts you can provide will be quite helpful!

Cheers,
Dan