Unsynchronized access to connection object from streams.

[Lukasa]

During stream creation, streams are provided access to the underlying connection object without its lock wrapper. This allows streams to perform unsynchronised access to the connection object, which is BAD.

While we’re here we should evaluate whether streams should be using _send_outstanding_data instead of the _send_cb, and indeed whether they should be just handed a reference to the connection instead of the various CBs.

[fredthomsen]

Seems that passing in the LockedObject and not just the underlying H2Connection where Stream is constructed in the HTTP20Connection solves the first problem. Perhaps I am not completely understanding and you can point out the error of my ways.

As far as the passing in a reference, I assume you mean to the HTTP20Connection. That seems very much like a layering/design pattern violation no? Is there some reason why the callbacks are frowned upon?

[Lukasa]

@fredthomsen You're entirely right about the first problem, that would solve it. You'd also need to rewrite a lot of code to use with statements to grab the lock, but that's ok.

As to passing in a reference, there's no way around it: there's a layering violation going to happen here somewhere because connection objects call streams and then streams call connection objects. The only way around that is to heavily refactor the code so that streams no longer need to call the connection at all, which is certainly do-able. Otherwise, one way or another, we need to pass something to the stream. The big problem right now is that the stream doesn't have access to send_outstanding_data, which I'd much rather they used than the current send_cb that they do use.

[fredthomsen]

This feel icky. Made a PR for now.