Sorry, I meant a constructor that takes a tid_t seems useful.
On Fri, Aug 29, 2014 at 12:38 PM, Zachary Turner <[email protected]> wrote: > BTW, one thing I was stuck on in my original patch. It seems I can go > from a thread_t to a tid_t on MacOSX via pthread_threadid_np. How do I go > the other direction? HostThreadBase is going to store a thread_t. But a > method that returns a tid_t seems generally useful. > > > On Fri, Aug 29, 2014 at 12:34 PM, <[email protected]> wrote: > >> >> > On Aug 29, 2014, at 12:33 PM, Zachary Turner <[email protected]> >> wrote: >> > >> > I'll proceed as discussed in the earlier thread where I listed the >> interfaces, minus the Suspend / Resume. I'll keep an eye on this thread >> though in case anything else comes up. Thanks for trudging through this >> with me! >> >> Sounds great! More thanks always go to the fellow actually doing the >> work! >> >> Jim >> >> > >> > >> > On Fri, Aug 29, 2014 at 12:31 PM, <[email protected]> wrote: >> > Yes, that sounds right. >> > >> > Jim >> > >> > > On Aug 29, 2014, at 12:30 PM, Zachary Turner <[email protected]> >> wrote: >> > > >> > > Thinking about it some more, removing SuspendThread and ResumeThread >> seems reasonable. Functionality beyond what is required for dealing with >> own-process threads can be in a different abstraction which is backed by a >> HostThread through containment. >> > > >> > > >> > > On Fri, Aug 29, 2014 at 12:12 PM, <[email protected]> wrote: >> > > Yes, I think there's enough reason to expect we'll use HostThread in >> ways the C++11 standards committee didn't think of that keeping a wrapper >> is useful. Also, Hosts mostly have to have fairly good thread libraries >> now-a-days, but they don't necessarily have to have good C++11 std::thread >> implementations, so insulating ourselves here is probably not a bad idea. >> OTOH, Greg has been making rumblings about backing the Host thread >> functionality by std::thread's ever since we decided to require C++11. >> > > >> > > Among other teenie reasons for keeping the wrapper, std::thread's >> don't have "set name". That may seem trivial, but we get crash logs from >> Xcode where Xcode is running many lldb Debugger sessions simultaneously >> (the Swift Playground feature is one among many causes of this.) These >> CrashLogs were really hard to read till we got both Xcode and lldb to name >> all the threads that belong to a given process with the PID of the >> process. I would not like to lose this feature... >> > > >> > > Jim >> > > >> > > >> > > > On Aug 29, 2014, at 11:59 AM, Zachary Turner <[email protected]> >> wrote: >> > > > >> > > > Actually, disregard my last message. I agree with you, the things >> you do when you manipulate threads in a debuggee are different. So for >> that you have some other abstraction, but that other abstraction can still >> be *backed by* a HostThread >> > > > >> > > > >> > > > On Fri, Aug 29, 2014 at 11:54 AM, Zachary Turner < >> [email protected]> wrote: >> > > > 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, <[email protected]> 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 <[email protected]> >> 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, <[email protected]> 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 < >> [email protected]> 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, <[email protected]> wrote: >> > > > > > >> > > > > > > On Aug 29, 2014, at 10:09 AM, Zachary Turner < >> [email protected]> wrote: >> > > > > > > >> > > > > > > On Fri, Aug 29, 2014 at 9:54 AM, <[email protected]> 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. >> > > > > > >> > > > > > >> > > > > >> > > > > >> > > > >> > > > >> > > > >> > > >> > > >> > >> > >> >> >
_______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
