[Lldb-commits] [PATCH] C++ API tests review request
Malea, Daniel
daniel.malea at intel.com
Fri May 3 11:37:57 PDT 2013
Oh, how embarrassing; I am slow today and was pointing my test script to a
stale binary where I had re-enabled the private lock..
After a proper rebuild, the resume case is now passing! I have to look at
the other ones a little closer, I think there's other problems with the
test cases that is causing them to fail. In any case, no deadlock :)
Thanks,
Dan
On 2013-05-03 1:59 PM, "jingham at apple.com" <jingham at apple.com> wrote:
>I'm a little confused here. Looks like you are stalling in trying to get
>the "private run lock" that Greg introduced a little while back. That is
>currently #ifdef'ed __APPLE__, at least in my sources. That definitely
>has some problems still, and is what I'm working to sort out right now.
>But it shouldn't be active for you if you are on Linux.
>
>Jim
>
>On May 3, 2013, at 10:46 AM, "Malea, Daniel" <daniel.malea at intel.com>
>wrote:
>
>> So I tried your suggestions of using SBTarget.Launch(listener,..) to
>> attach a listener at construction, and also tried using
>> SBDebugger.GetListener() after launching the process with
>>LaunchSimple(),
>> but these approaches don't yield good results. Specifically, I'm now
>> seeing every test (that uses listeners) deadlock:
>>
>> #0 pthread_rwlock_wrlock () at
>> ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_rwlock_wrlock.S:83
>> #1 0x00007f150e30a5fe in lldb_private::ReadWriteLock::WriteLock
>> (this=0x1958b50) at ../tools/lldb/include/lldb/Host/ReadWriteLock.h:77
>> #2 0x00007f150e304c46 in lldb_private::Process::PrivateResume
>> (this=0x1958200) at ../tools/lldb/source/Target/Process.cpp:3282
>> #3 0x00007f150e2fff7c in lldb_private::Process::Resume (this=0x1958200)
>> at ../tools/lldb/source/Target/Process.cpp:1677
>> #4 0x00007f150e08b732 in lldb::SBTarget::Launch (this=0x7fff2dc86d60,
>> listener=..., argv=0x0, envp=0x0, stdin_path=0x0, stdout_path=0x0,
>> stderr_path=0x0, working_directory=0x195ca30
>> "/home/daniel/dev/llvm/tools/lldb/test/api/multithreaded",
>>launch_flags=0,
>> stop_at_entry=false, error=...)
>> at ../tools/lldb/source/API/SBTarget.cpp:712
>> #5 0x00007f150e08b171 in lldb::SBTarget::LaunchSimple
>> (this=0x7fff2dc86d60, argv=0x0, envp=0x0, working_directory=0x195ca30
>> "/home/daniel/dev/llvm/tools/lldb/test/api/multithreaded") at
>> ../tools/lldb/source/API/SBTarget.cpp:603
>> #6 0x0000000000404115 in test (dbg=..., args=...) at
>> test_breakpoint_callback.cpp:48
>> #7 0x0000000000402f78 in main (argc=2, argv=0x7fff2dc86f48) at
>> driver.cpp:30
>>
>>
>>
>>
>> It looks quite similar to the other deadlock we were discussing
>>yesterday
>> in the context of that ThreadPlanBase patch. Anyways, I have not yet
>>been
>> able to enable LLDB logging in these test cases, but I imagine it might
>>be
>> useful to see what is going awry here. I have not yet had a chance to
>>try
>> it on Mac OS X yet, but that is my next step; it should help clarify if
>> it's something related to POSIX/Linux plugins (like the WillResume
>> function Greg mentioned) in generic code.
>>
>> I'm nonetheless attaching the updated patch that attaches a listener on
>> Process construction in listener_test.cpp:49. Maybe you can see
>>something
>> else wrong with it?
>>
>> Cheers,
>> Dan
>>
>>
>>
>> On 2013-05-03 12:03 PM, "jingham at apple.com" <jingham at apple.com> wrote:
>>
>>>
>>> On May 3, 2013, at 8:51 AM, "Malea, Daniel" <daniel.malea at intel.com>
>>> wrote:
>>>
>>>> I see; that makes sense. I was copying the approach used by
>>>> examples/python/process_events.py:
>>>>
>>>> 178 listener = lldb.SBListener("event_listener")
>>>>
>>>> 179 # sign up for process state change events
>>>>
>>>> 180 process.GetBroadcaster().AddListener(listener,
>>>> lldb.SBProcess.eBroadcastBitStateChanged)
>>>>
>>>>
>>>>
>>>> Which seems to work, but now I'm wondering if that's correct? Should
>>>>it
>>>> be
>>>> fixed, or is there some magic that allows that particular python code
>>>>to
>>>> create a new listener and register it?
>>>
>>> I was just about to tell you not to copy that test case.
>>>
>>> We put in some asserts to make sure we weren't unlocking the unlocked
>>> write end of the run lock (they are in TOT but turned off at present.)
>>> With them turned on this test asserts,. I'll have to fix it as I go
>>> through and clean up all the places where these asserts fail.
>>>
>>>>
>>>> I will try using the debugger's listener instead of my own and update
>>>> the
>>>> patch, as well as the test pass/fail status.
>>>>
>>>
>>> Yes, that would be best. At some point it would be great to get back
>>>to
>>> allowing multiple listeners for the Process events. But unlike most
>>> other events in lldb, things happen when you fetch the events from the
>>> event queue, so it is confusing when various agents are allowed to pull
>>> then off. If we want to do this, we should probably make it so that
>>>the
>>> one of the listeners is the driver, and the others wait to get notified
>>> till the event has been delivered to the driving listener or something
>>> like that. But I don't think this is a terribly important task.
>>>
>>> Jim
>>>
>>>> Thanks,
>>>> Dan
>>>>
>>>>
>>>>
>>>> On 2013-05-03 11:36 AM, "jingham at apple.com" <jingham at apple.com> wrote:
>>>>
>>>>> BTW, just to give a little background on this. If there are no
>>>>> listeners
>>>>> for an event, Broadcasters in lldb just discard them. That's useful,
>>>>> for
>>>>> instance if nobody cares about breakpoint changed events, there's no
>>>>> reason for the target to hold onto all of them just in case somebody
>>>>> might later care. But that means if you start up a process with no
>>>>> listener, then there's a race condition with whether your listener
>>>>>will
>>>>> be in time to get the initial stopped event. If it gets added after
>>>>> the
>>>>> process is launched and stopped, for instance, it could miss it.
>>>>>
>>>>> I was going to solve that by having each Broadcaster have a "Prime a
>>>>> new
>>>>> listener with relevant events" method, so that the Process
>>>>>Broadcaster
>>>>> would hold onto all the events it had gotten till some listener
>>>>>signed
>>>>> up
>>>>> with it. In fact, the method is still there
>>>>> (Broadcaster::AddInitialEventsToListener.) But Greg didn't like that
>>>>> in
>>>>> the case of the Process for what are I am sure good reasons though I
>>>>> can
>>>>> no longer recall them. So we decided to just force the Process to be
>>>>> created with a listener.
>>>>>
>>>>> Jim
>>>>>
>>>>> On May 3, 2013, at 8:29 AM, jingham at apple.com wrote:
>>>>>
>>>>>> The way the SB API's work for creating processes, if you don't
>>>>>> register
>>>>>> a listener at construction, your Process gets hooked up to the
>>>>>> Debugger's listener, which you can get with SBDebugger.GetListener.
>>>>>> So
>>>>>> you do have two. Better to either construct it with a listener
>>>>>> explicitly, or use the debugger's.
>>>>>>
>>>>>> Jim
>>>>>>
>>>>>> On May 3, 2013, at 5:50 AM, "Malea, Daniel" <daniel.malea at intel.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Thanks for the heads up; will keep this in mind. The tests only
>>>>>>> register one SBListener with each process; but it's not done at
>>>>>>> construction, but via:
>>>>>>>
>>>>>>> Process.GetBroadcaster.AddListener(listener,
>>>>>>> eBroadcastBitStateChanged);
>>>>>>>
>>>>>>> However, I should note that the tests call the listener's
>>>>>>> waitForEvent() from another thread, which I imagine is a popular
>>>>>>>use
>>>>>>> case for frontends.
>>>>>>>
>>>>>>>
>>>>>>> Any comments appreciated; the (svn) patch is attached to the
>>>>>>>previous
>>>>>>> email (addressed to Greg/the list) in this thread.
>>>>>>>
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Dan
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: jingham at apple.com [mailto:jingham at apple.com]
>>>>>>> Sent: Thursday, May 02, 2013 6:04 PM
>>>>>>> To: Malea, Daniel
>>>>>>> Cc: lldb-commits at cs.uiuc.edu
>>>>>>> Subject: Re: [Lldb-commits] [PATCH] C++ API tests review request
>>>>>>>
>>>>>>> One thing to be wary of with the process and listeners is that
>>>>>>> although it is possible to have two listeners listening to process
>>>>>>> events, and at some point that should work, at present it doesn't
>>>>>>> work
>>>>>>> all that well. For the nonce, if you are going to write test cases
>>>>>>> that involve consuming process events, just use the listener that
>>>>>>>you
>>>>>>> passed to the process when you created it.
>>>>>>>
>>>>>>> Jim
>>>>>>>
>>>>>>>
>>>>>>> On May 2, 2013, at 2:55 PM, "Malea, Daniel"
>>>>>>><daniel.malea at intel.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> I'm trying to nail down exactly what multithreaded problems there
>>>>>>>> are
>>>>>>>> with the LLDB API, so I wrote some tests that use it via C++. The
>>>>>>>> same
>>>>>>>> stuff could have been done from Python, but I figured since
>>>>>>>>there's
>>>>>>>> so
>>>>>>>> few C++ tests (one that I found "check_public_api_headers") I
>>>>>>>> figured
>>>>>>>> I'd write some to keep things interesting.
>>>>>>>>
>>>>>>>> When someone has a moment, could you provide some feedback, as I'm
>>>>>>>> seeing 3 out of 4 tests fail on Linux/Mac OS X - wondering if
>>>>>>>>these
>>>>>>>> are LLDB bugs I'm uncovering, or more likely, that the tests are
>>>>>>>> using
>>>>>>>> the API incorrectly.
>>>>>>>>
>>>>>>>> Here's a brief description of what the tests do:
>>>>>>>>
>>>>>>>>
>>>>>>>> 1. Test that a function registered with
>>>>>>>>SBBreakpoint.SetCallback()
>>>>>>>> is called when a breakpoint is hit. -- the function is not invoked
>>>>>>>> 2. Test that an SBListener registered with a process receives an
>>>>>>>> event.
>>>>>>>> 3. Test that a process retrieved from an SBEvent can be used to
>>>>>>>> query thread/frame information - retrieving thread/frame works ok,
>>>>>>>> but querying a function name does not 4. Test that a process can
>>>>>>>> be
>>>>>>>> resumed from a thread that did not start it - This one's weird:
>>>>>>>> SBProcess::GetState() returns eStateStopped() but calling
>>>>>>>> Process::Resume() fails with error "process is running".
>>>>>>>>
>>>>>>>> Currently, only #2 passes, which is the simplest case. There's a
>>>>>>>> bunch of boiler plate code in there that is not too interesting.
>>>>>>>>The
>>>>>>>> meat of the test logic are in the .cpp files that are prefixed
>>>>>>>>with
>>>>>>>> "test_".
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks a bunch,
>>>>>>>> Dan
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> lldb-commits mailing list
>>>>>>>> lldb-commits at cs.uiuc.edu
>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>>
>>
>> <test_api_multithreaded_v2.patch>
>
More information about the lldb-commits
mailing list