Re: [PATCH] client-side redesign pt1 - Comm::TcpReceiver

tis 2014-01-21 klockan 22:45 -0700 skrev Alex Rousskov:

> All the TCP clients and servers you are willing to include (as future
> TcpReceiver kids) in the current project scope have at least one thing
> in common -- they all read and write protocol messages over [persistent]
> TCP connections. Why do you think it is a good idea to focus on just the
> reading part so much? If you want a common parent for all those agents,
> why not handle both actions in that common parent, especially if you
> already find it necessary to add a little bit of "send" part into the
> TcpReceiver?

I haven't looked in detail at what is being done, but also keep in mind
that if you layer TLS ontop of a TCP connection then the TLS layer needs
both read & write events to the TCP connection.

From a code design perspective it should be the same interface for
application protocol data for both TCP and TLS.

For some food of thought I would throw SCTP into that mix as well, which
adds another layer ontop of the socket to track which SCTP channel the
application level connection runs on. So we might have aplication /
TLS / SCTP channel / SCTP(protocol).

Any design that tries to make application level code (i.e. http/ftp
protocol handlers etc) needing to be aware of TcpReceiver/Sender is
plain wrong imho.

> If tomorrow we have both TcpReceiver and TcpSender, and all their kids
> have both classes as parents, with complex inter-parent dependencies
> that are already showing in TcpReceiver, we are doing something wrong.

We should only have channels and listeners at application protocol level
code (HTTP, ftp, whatever). To most code it does not matter what
transport these use.

Details of each transport implementation is more free. Not so much need
to argue on details there. But should not be visible in other code.

> > It only exists because tunnel.cc takes over the FD. Really nasty (the
> > dropped data race condition).

yes, CONNECT requests will be a mess. But as above, should take over the
connection. Any application payload already read need to be either
handed back to the connection or given to the CONNECT forwarder.

> Let's call this method stopReadingXXX() so that it is clear that any
> caller is "doing it wrong" and to discourage use.

Yes. What is needed is to stop accepting pipelined requests after a
CONNECT. Stopping reading is justa brute-force way of accomplishing
that.

> I do not know or do not remember, sorry. The whole "delay pools cannot
> deal with single-byte buffers" is a delay pools bug, outside this
> project scope.

I think it's an ancient parser issue which hopefully is long since
fixed.
Received on Wed Jan 22 2014 - 09:33:01 MST