[Lldb-commits] [PATCH] C++ API tests review request
Malea, Daniel
daniel.malea at intel.com
Fri May 3 10:46:14 PDT 2013
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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test_api_multithreaded_v2.patch
Type: application/octet-stream
Size: 17120 bytes
Desc: test_api_multithreaded_v2.patch
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20130503/d9fc922d/attachment.obj>
More information about the lldb-commits
mailing list