On Mon, Jun 3, 2019 at 11:06 PM Mark Thomas <ma...@apache.org> wrote:
> On 03/06/2019 21:45, Rémy Maucherat wrote: > > On Mon, Jun 3, 2019 at 10:05 PM Mark Thomas <ma...@apache.org > > <mailto:ma...@apache.org>> wrote: > > > > On 03/06/2019 19:54, Rémy Maucherat wrote: > > > > <snip/> > > > > > Ok, that's completely different ;) I tried to be careful with > > that, but > > > the algorithm changes to non blocking, so obviously there were some > > > risks involved. I'll have a quick look at it, but you can go ahead > > with > > > your window update fix and tag, this NIO2 problem is likely not a > > super > > > critical issue. > > > > I think I am making progress. The issue appears to stem from the > > connection preface being processed in-line on Linux and OSX but not > > in-line on Windows. > > > > I'm probably missing something because my fix so far (and it isn't > > complete) is making changes to the Http2UpgradeHandler API. > > > > > > Ok, that's very consistent with your concurrency issue. The problem may > > be with the upgradeHandler.upgradeDispatch(SocketEvent.OPEN_READ); at > > line 124 in Http2AsyncParser, which shouldn't be called in some cases. > > I'm trying to experiment there first, but since I don't get the problem > > it's hard to figure it out. > > Making changes to Http2UpgradeHandler could be ok since the preface was > > blocking and I already made some changes to accommodate async > > (processConnection -> processConnectionCallback). > > How about this as an idea: > > --- a/java/org/apache/coyote/AbstractProtocol.java > +++ b/java/org/apache/coyote/AbstractProtocol.java > @@ -905,6 +905,10 @@ > } > } > } > + // The handler will initiate any further I/O > + if (wrapper.hasAsyncIO()) { > + state = SocketState.LONG; > + } > } > } while ( state == SocketState.UPGRADING); > > > Essentially, it is saying if the upgrade handler is async, it will take > care of triggering any further reads that may be necessary. > > Initial test results are promising. > Worth trying. Rémy
