On 22/01/2014 10:32 p.m., Henrik Nordström wrote: > 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.
Yes. I was loosely thinking of completing this TcpAgent then working on TlsAgent, etc. with the same interface methods. COAPS and UDP I/O streams are also on the wishlist as other *Agent types. With Alex proposal to merge the Comm::Write API into these fully I think we have a good design to go forward with that can be spread across all those transport types relatively easily. > >> 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. > Precisely. The long-term plan is to have the clients/ and servers/ classes using these FooAgent classes or their children for abstract access to buffered byte streams instead of dealing with any connection specifics themselves. > >>> 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. > Thank you. So we have several possible reasons. At least delay pools is likely to be around for some time yet. So I'm going to leave this as-is for the purposes of this patch. It is not exactly harmful to any design as-is just nasty looking code. Amos
