python-hyper / hyper

HTTP/2 for Python.
http://hyper.rtfd.org/en/latest/
MIT License
1.05k stars 193 forks source link

Jetty and Hyper #207

Closed judepereira closed 8 years ago

judepereira commented 8 years ago

Hey @Lukasa, continuing off Twitter: I compared the tcpdump between Jetty and Hyper, with Jetty and Go. The connection was being terminated just after the handshake. Then it struck it, that the self-signed certificate must be imported into the Java keystore (I had done this just before I tried Go, but didn't do so with Python, as I was expecting Jetty to throw some CertificateException).

Okay, so now running the Python script after importing the certificate into the keystore, things seem to start working a bit.

Jetty can complete a request successfully! However, at the server bit, every request throws the following exception:

Exception in callback _SelectorSocketTransport._read_ready()
handle: <Handle _SelectorSocketTransport._read_ready()>
Traceback (most recent call last):
  File "/usr/local/lib/python3.5/site-packages/asyncio-3.4.3-py3.5.egg/asyncio/events.py", line 120, in _run
    self._callback(*self._args)
  File "/usr/local/lib/python3.5/site-packages/asyncio-3.4.3-py3.5.egg/asyncio/selector_events.py", line 669, in _read_ready
    self._protocol.data_received(data)
  File "/usr/local/lib/python3.5/site-packages/asyncio-3.4.3-py3.5.egg/asyncio/sslproto.py", line 499, in data_received
    self._app_protocol.data_received(chunk)
  File "apns-server-http2.py", line 57, in data_received
    self.conn.reset_stream(event.stream_id)
  File "/usr/local/lib/python3.5/site-packages/h2-2.1.1-py3.5.egg/h2/connection.py", line 689, in reset_stream
    frames = stream.reset_stream(error_code)
  File "/usr/local/lib/python3.5/site-packages/h2-2.1.1-py3.5.egg/h2/stream.py", line 774, in reset_stream
    self.state_machine.process_input(StreamInputs.SEND_RST_STREAM)
  File "/usr/local/lib/python3.5/site-packages/h2-2.1.1-py3.5.egg/h2/stream.py", line 108, in process_input
    return func(self, previous_state)
  File "/usr/local/lib/python3.5/site-packages/h2-2.1.1-py3.5.egg/h2/stream.py", line 286, in send_on_closed_stream
    raise StreamClosedError(self.stream_id)
h2.exceptions.StreamClosedError: 17

I'm sure that this is something silly that I'm doing in the Python script (same one from this gist).

Any pointers here?

judepereira commented 8 years ago

Note: I can read the response body in Jetty completely.

Lukasa commented 8 years ago

So the problem here is in your handling of DataReceived. =)

Specifically, when your code receives data it forcefully closes the stream. The thing is, I saw your test client: it sends a POST with data in it! So that's probably not what you want.

In this instance, your DataReceived code should probably instead call conn.increment_flow_control_window twice: once with no stream ID, once with the ID of the stream in question, and otherwise do nothing. =)

Lukasa commented 8 years ago

Oh, by the way, and I know this is confusing: the actual project you're using here is tracked in the repo at python-hyper/hyper-h2. This is a separate but related library with a similar name. I screwed the naming up a bit here. ;)

judepereira commented 8 years ago

Got it! What value should I use as the window size (the first parameter to increment_flow_control_window)?

Also, if I skip increment_flow_control and reset stream while processing the DataReceived event, everything seems to work (no exceptions anywhere), however I think I'm not closing the stream at all (it keeps incrementing and not reusing - reuses after I restart my Jetty client).

However, at the same time I don't think that I should be doing an explicit stream reset (as the StreamEnded event is fired last).

I've updated my gist, and here's the diff.

The problem is that after the first 2,258 successful requests, Jetty can't read the response from Hyper. It just freezes at trying to read the response from the network (a thread dump from Java confirms this).

What could I be doing wrong?

Lukasa commented 8 years ago

For your parameter to increment_flow_control you should use event.flow_controlled_length.

So the reason your app is freezing is the same reason you can't just leave the DataReceived event unhandled. HTTP/2 maintains a flow control window for every stream, and one for the connection. That window shrinks for every DATA frame received. Hyper-h2 does not automatically increment the window for you (because it wants to enable intelligent flow control strategies), so if you don't call increment_flow_control_window eventually the window size drops to zero and no data can be sent until it's reopened. Add the increment and you'll be fine.

As to the ever-incrementing stream IDs, that's just how HTTP/2 works: each stream ID is only used once per connection.

judepereira commented 8 years ago

Thanks for the explanation @Lukasa! That helped me understand it better. Calling increment_flow_control_window for the connection made it work perfectly!

However, when I call it for the stream, it throws a stream ended error. All in all, just adding calling it for the connection, seems to work just well (100K was my benchmark).

Thanks for all your help!