On 25.01.2014 21:16, rhuij...@apache.org wrote: > Author: rhuijben > Date: Sat Jan 25 20:16:57 2014 > New Revision: 1561387 > > URL: http://svn.apache.org/r1561387 > Log: > Following up on r1531610 and related revisions, extend the ra-svn tunnel > creation callback to allow providing Subversion streams instead of only > apr_file_t * handles to allow hooking in third party libraries without > requiring external applications and/or advanced multithread synchronization > in any application that wants to use this.
You do realize that with this change, any tunnel implementation will always violate the svn_stream API promises? * The read and write handlers accept length arguments via pointer. * On entry to the handler, the pointed-to value should be the amount * of data which can be read or the amount of data to write. When the * handler returns, the value is reset to the amount of data actually * read or written. Handlers are obliged to complete a read or write * to the maximum extent possible; thus, a short read with no * associated error implies the end of the input stream, and a short * write should never occur without an associated error. Note especialy this: "a short read with no associated error implies the end of the input stream". This simply is not true for the ra_svn protocol: the client can expect to always write the whole buffer to the server, but reading less than the client's allocated buffer does *not* mean that the server has closed the connection. The JavaHL implementation of the tunnel callbacks is a good example: it can use apr_file_write_full on the request end of the pipe, but only plain apr_file_read on the response end, otherwise you get a deadlock when the server has sent a response, and is waits for the client, while the client is waits for its read buffer to fill up. You'll note that read_handler_apr (which used by streams returned from svn_stream_from_aprfile2) use svn_io_file_read_full2; in other words, if you use this changed tunnel handler to integrate libssh2 into the Windows build, you're going to get deadlocks on svn+ssh://; and so will any other callback implementation (except the Java one, which uses proper pipes and does not use full-read). Our stream API is quite useless for handling ra_svn, because it does not provide explicit end-of-stream detection; on other words, it's not a real stream API, it assumes that there's always a file at the end of the stream chain; which is not even close to being true in ra_svn. And a last note: changing the pipe to a stream doesn't make it easier for implementers, even your libssh2 example. You're just moving the responsibility for managing the pipe somewhere else. -1 for all the reasons stated above, please revert. — Brane -- Branko Čibej | Director of Subversion WANdisco // Non-Stop Data e. br...@wandisco.com
