[Lldb-commits] [PATCH] Implements a HostThread class.

Zachary Turner zturner at google.com
Fri Aug 29 11:54:01 PDT 2014 

If that's the case, is there any reason to not just use std::thread
directly?


On Fri, Aug 29, 2014 at 11:51 AM, <jingham at apple.com> wrote:

> So thread_t says:
>
> //  lldb::thread_t          The native thread type for spawned threads on
> the system
>
> I am pretty sure that was only ever meant to be used for threads inside of
> the running lldb.  It seems to me that it will make things very confusing
> to try to use it for random threads in some other process.  The way you
> manipulate your own threads, and the things you can do with them, are very
> different from the way you manipulate threads in the debugee, and what is
> desirable.  Unless there's some compelling reason that I don't see right
> now, I'd like to keep these two concepts separate.
>
> Jim
>
>
> > On Aug 29, 2014, at 11:35 AM, Zachary Turner <zturner at google.com> wrote:
> >
> > I mentioned this a few times earlier, but admittedly the thread has
> grown pretty long.
> >
> > HostThread is a replacement for thread_t.  Anywhere you use a thread_t,
> you can use a HostThread.  This applies obviously to threads inside the
> debugger itself, but also to any other threads on the machine.  In the case
> of debugging process P locally, for example, it might be used when viewing
> or manipulating the threads of P.  Even in remote debuggign scenarios, the
> debugserver might use a HostThread to manipulate threads in the inferior.
> There's already a method in Host called FindProcessThreads, for example.
> Instead of returning a list of thread ids, it could return a list of
> HostThreads.  The size of the class should not be much (if any) larger than
> an lldb::thread_t, it just has extra methods for convenience.
> >
> >
> > On Fri, Aug 29, 2014 at 11:28 AM, <jingham at apple.com> wrote:
> > One thing I am unclear about, I had thought that HostThread in
> particular would be used for threads IN lldb, but not for handling threads
> in a program lldb is debugging when on that Host.  So for instance, Suspend
> & Resume don't seem necessary or desirable when dealing with threads lldb
> is using for implementing lldb, and Launch is possible but quite tricky and
> not required for remote threads on the same Host.
> >
> > What is the actual story here?
> >
> > Jim
> >
> >
> > > On Aug 29, 2014, at 11:19 AM, Zachary Turner <zturner at google.com>
> wrote:
> > >
> > > What do you think about this?  I implement the abstract base class.
> I'll give it the following methods:
> > >
> > > Pure virtual:
> > > * Launch
> > > * Join
> > > * Cancel
> > > * GetState (enumerated value : invalid, running, exited, suspended,
> cancelled)
> > > * Suspend
> > > * Resume
> > > * GetThreadResult
> > >
> > > Virtual with default implementations:
> > > * GetName
> > > * SetName
> > >
> > > Cancel will return an lldb::Error if cancellation is not supported
> (for example because we're on windows and we don't own the thread
> routine).  If you think of any other methods that should go here, let me
> know.
> > >
> > > Initial patch will go in like this, with no code yet updated to use
> the new HostThread.  In a subsequent patch, I will change thread_t's to
> HostThreads, updating code as necessary.  We can evaluate the decision
> about the typedef vs. the base class at this point, when we see what (if
> any) impact this will have on whether specific methods will be accessed
> from generic code.  At this point, if there's still disagreement, I can
> even make two separate patches - one that does it each way, so that we can
> compare actual code that uses both methods.
> > >
> > >
> > > On Fri, Aug 29, 2014 at 11:01 AM, <jingham at apple.com> wrote:
> > >
> > > > On Aug 29, 2014, at 10:09 AM, Zachary Turner <zturner at google.com>
> wrote:
> > > >
> > > > On Fri, Aug 29, 2014 at 9:54 AM, <jingham at apple.com> wrote:
> > > >
> > > > > SystemLog
> > > > > ThreadDetach
> > > > > ThreadCancel
> > > >
> > > > There must be some way on Windows to tell a thread to exit at the
> next cancellation point?  Can you really not cancel a worker thread's
> operation?
> > > >
> > > > Not an arbitrary thread, no.  You can forcefully terminate it (with
> caveats, and it may not work and/or have unexpected behavior, so it is
> strongly discouraged), but generally if you want to gracefully cancel a
> thread, the thread has to have specific code in its run loop to make that
> possible.  So it might work for threads that we create inside the lldb
> process, since we control the thread routine, but it wouldn't be meaningful
> for threads in other process.  The reason for this discrepancy is that
> there is no concept of signals on Windows, which I assume is how thread
> cancellation is implemented behind the scenes on posix platforms.
> > >
> > > I don't think thread cancellation requires signals for same process
> thread cancellation for pthreads, but it does use kernel support.  The
> system defines "cancellation points" - e.g. select, read, etc. where a
> thread is likely to sit waiting, and pthread_cancel just marks the target
> thread so that if it is in a cancellation routine (they're pretty much all
> kernel traps) it gets killed off, otherwise the request is postponed till
> the thread enters a cancellation point and then it gets killed.   I don't
> think you would need signals for this.
> > >
> > > But it is something we need to do for all our ports, since we rely on
> this for shutting down the Process worker threads cleanly.  Since we only
> really care about canceling threads we create, I think it would be okay for
> ThreadCancel to be a no-op for other threads.
> > >
> > > >
> > > >
> > > > > GetAuxvData
> > > > >
> > > > > Before my original HostInfo refactor, it also had these methods:
> > > > >
> > > > > GetUserID
> > > > > GetGroupID
> > > > > GetEffectiveUserID
> > > > > GetEffectiveGroupID
> > > > >
> > > > > Those methods were just as accessible to anyone writing generic
> code.  They are *less* accessible now, because they are on HostInfoPosix.
> And this happened without needing to introduce any platform specific
> pre-processor defines into generic LLDB.  There was maybe one exception,
> which Jason Molenda pointed out earlier, which was the GetOSBuildString.
> And that only happened because GetOSBuildString is not a generic concept!
>  The design worked exactly as intended, exposing a place where code that
> was intended to be generic actually wasn't as generic as it thought it
> was.  Everywhere else, the GetUserID, GetGroupID, etc methods were only
> being called from specific code (which is how it should work), but after my
> change this is now enforced by the compiler.
> > > > >
> > > > >
> > > >
> > > > But again, if I'm writing code it seems like a real pain to have to
> answer the question "is this an interface I can use in generic code" be:
> > > >
> > > > For now you can if it exists in all the HostThreadXXX.h files, but
> of course if somebody introduces another platform and decides that they
> don't want to implement this function then you can't use it anymore and
> have to put
> > > >
> > > > #if defined
> > > >
> > > > guards around the usage.  Instead, we should look at the host
> interfaces and say there are three classes of things:
> > > >
> > > > 1) Things you must implement in order to port lldb to a host
> > > >     These should be pure virtual methods in HostFeature.h
> > > > 2) Things you can optionally implement, but there's a reasonable
> "couldn't do that" fallback
> > > >     These should be virtual methods in HostFeature.h
> > > > 3) Things that are purely host specific.
> > > >     These should be methods in HostFeatureMyHost.h, and can only be
> used in HostOtherFeatureMyHost.h, but never in generic code.
> > > >
> > > > This would make the job of folks working in generic code clear, and
> also make it obvious to the next porter (OpenVMS, anyone?) what they have
> to do.
> > > >
> > > > I'm ok with doing all of this.  I'm all for having the compiler
> catch things for you, and if one of the things it can catch for you is "you
> need to implement this method" then that's great.  That said, I'm still
> rather fond of the idea of typedefing HostFeature to HostFeatureMyHost and
> then having everyone use HostFeature.  No matter if you do it or don't,
> it's still equally easy to write specific code from generic code.  You
> could just do this:
> > > >
> > > > #if defined(__APPLE__)
> > > > ((HostFeatureMacOSX&)feature).AppleSpecificMethod();
> > > > #endif
> > > >
> > > > as opposed to this
> > > >
> > > > #if defined(__APPLE__)
> > > > feature.AppleSpecificMethod();
> > > > #endif
> > > >
> > >
> > > My feeling about this is since you'd actually have to do:
> > >
> > > #if defined(__APPLE__)
> > > #include "lldb/Host/windows/HostFeatureMacOSX.h"
> > > (((HostFeatureMacOSX&)feature).AppleSpecificMethod();
> > > #endif
> > >
> > > I hope at that point you'd know you've gone off the reservation.
> > >
> > > Jim
> > >
> > >
> > >
> > > > In both cases though, the person has felt strongly enough about it
> to put it in a pre-processor guard, so the decision has already been made.
> And the second method has the benefit that when you're writing specific
> code (which I anticipate to write alot of for Windows), you just always
> have the type you need.
> > >
> > >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140829/4e3634c4/attachment.html>

More information about the lldb-commits mailing list