[Lldb-commits] [PATCH] Move ConnectionFileDescriptor to posix-subfolder in preparation for implementing on Windows

[Zachary Turner]

Actually scratch this.  After looking further, select() and
WaitForMultipleObjects() are too different in the types of objects they can
operate on.  Don't think this would work well.

On Tue, Sep 30, 2014 at 2:58 PM, Zachary Turner <zturner at google.com> wrote:

> Let me bounce another idea I had off you.  One thing I thought of was
> making a new interface called Waitable.  It has a pure virtual method
> called GetWaitHandle().  On posix this just returns the fd, on Windows it
> returns a HANDLE.  Now, instead of CFD being sort of like a multiplexer
> (one class that what it creates depends on your connection string), you
> make different classes for each type of connection.  You have a
> FileConnection, a TcpConnection, a ListeningConnection, a
> NamedSocketConnection, etc.  Each of these inherit WaitableConnection,
> which is itself just an empty class that inherits both Connection and
> Waitable.
>
> You have a factory method higher up that is analogous to the existing
> ConnectionFileDescriptor::Connect.  i.e. it takes the connection string,
> and returns an instance of the appropriate type (ListeningConnection, etc)
> depending on the connection string.
>
> Up in Host, you implement a Waiter abstraction which takes a list of
> Waitables.  On posix this is just going to call select(), on Windows it's
> going to call WaitForMultipleObjects.
>
> Back in Core, you have an InterruptibleConnection, which takes to its
> constructor a WaitableConnection, and provides an Interrupt method.  On
> posix, this interrupt is done via a pipe, on Windows via an event.  The
> only thing platform-specific now is the implementation of
> InterruptibleConnection.  I suppose this could be abstracted out even
> further by implementing an InterruptSignal in Host that is also a Waitable,
> then CFD itself could be implemented with no platform specific code.
>
> A little bit involved, so I don't mind doing it the easy way (basically
> just re-implementing CFD for both platforms).  But figured I would throw
> this out there.
>
> On Tue, Sep 30, 2014 at 2:44 PM, <jingham at apple.com> wrote:
>
>> I don't think it is a problem for something in Host to use something from
>> Core, or vice versa.  For instance lots of stuff in Core will want to use
>> mutexes or FileSpec's, but those come from Host.  That doesn't seem a
>> problem to me.  Roughly Core contains basic classes for lldb whose
>> implementation is not Host specific, and Host contains basic classes for
>> lldb whose implementation is Host specific.  But that makes them co-equal
>> and free to use one another.  Note, we haven't been entirely strict about
>> this division up to now (for instance it's a little weird that
>> ConnectionMachPort is in Core.)
>>
>> I start to get bugged if something in Core (or Host for that matter)
>> starts depending on a particular Host's implementation of that feature.
>> That bit should be hidden behind the generic Host interface.
>>
>> Jim
>>
>>
>>
>> > On Sep 30, 2014, at 2:25 PM, Zachary Turner <zturner at google.com> wrote:
>> >
>> > There's another issue with it being in Host, which is that it inherits
>> from Connection, which is in Core.  It seems there's already a cyclic
>> dependency between Host and Core, are we ok with this?  Granted, the
>> circular dependency exists presently, but if that happened on accident,
>> then we shouldn't make it worse.
>> >
>> > On Tue, Sep 30, 2014 at 2:23 PM, <jingham at apple.com> wrote:
>> > Seems to me that if it needs separate host implementations then it
>> should be in the Host specific directories.  I sympathize with leaving it
>> in Core as a promise that later on somebody will figure out how to pull
>> just the Host specific parts into some lower level abstraction, but in the
>> medium term that just makes the lldb structure less clear.
>> >
>> > However, if it turns out that there is a lot of code duplication
>> between the Windows and Posix versions of this class, it would be better to
>> keep the common code in core and use either inheritance or some other
>> pattern to just put the Host specific implementation parts in their
>> appropriate Host directories.
>> >
>> > Jim
>> >
>> >
>> > > On Sep 30, 2014, at 2:06 PM, Zachary Turner <zturner at google.com>
>> wrote:
>> > >
>> > > Good question.  Logically it seems a little too high level for Host
>> to me.  Conceptually speaking, CFD is just a wrapper around one of any
>> number of host primitives.  For example, it's a wrapper around a file, or a
>> socket, or a pipe, etc.  It just allows you to treat all those things using
>> the same interface.  Host seems more appropriate for providing lower-level
>> primitives.  It's kind of a grey area here though, because the
>> implementation of CFD uses select(), which doesn't lend itself to a nice
>> port on Windows, basically the whole thing has to be re-written.
>> > >
>> > > I brainstormed ways of lowering a select-like abstraction into Host
>> so that CFD could just be implemented without any platform specific stuff,
>> but it's not that easy since the semantics have some subtle differences
>> that make it difficult to present a generic interface.
>> > >
>> > > On Tue, Sep 30, 2014 at 1:42 PM, Ed Maste <emaste at freebsd.org> wrote:
>> > > I haven't looked closely at the proposed patch, but should CFD
>> instead migrate to source/Host/...?
>> > >
>> > > http://reviews.llvm.org/D5548
>> > >
>> > >
>> > >
>> >
>> >
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140930/89aa3ec0/attachment.html>