On Sat, Dec 16, 2017 at 6:13 PM, Mark Thomas <ma...@apache.org> wrote:
> On 15/12/2017 15:15, Rémy Maucherat wrote: > > On Fri, Dec 15, 2017 at 12:33 PM, Mark Thomas <ma...@apache.org> wrote: > > > >> On 01/12/2017 16:54, r...@apache.org wrote: > >>> Author: remm > >>> Date: Fri Dec 1 16:54:38 2017 > >>> New Revision: 1816887 > >>> > >>> URL: http://svn.apache.org/viewvc?rev=1816887&view=rev > >>> Log: > >>> Do not call onDataAvailable without data to read. I tried method with > >> less side effects, but they all caused problems elsewhere, while this > one > >> passes the testsuite and everything else I could test it with. > >> > >> This commit is what triggered the failure in TestUpgrade that Gump is > >> current complaining about for APR/native. > >> > >> I can reproduce this failure on OSX. I haven't tried on other platforms. > >> > >> The patch looks right to me. I'm wondering if this change has simply > >> exposed an issue somewhere in the APR/native connector. I'll take a > look. > >> > > Oops, sorry. I didn't see TestUpgrade failing on CI, so that was it for > > me, I usually don't test APR anymore. > > > > Since it fails for me too, I debugged it, and the server IO events are > the > > same for APR (3 onDataAvailable calls), with the data written being the > > same as well (2*16 bytes in onWritePossible). I'm not sure yet why the > > client fails to read it with the patch in place. > > Figured it out. > > EOF isn't handled correctly (likely my fault) so onAllDataRead() isn't > triggered and no data is sent to the client. The lack of data triggers > the test failure. > > Fix to follow shortly. > > I still need to investigate but I suspect some back-ports will be > required. > > Thanks for the fix, hopefully it'll be fine now. Rémy
