dedis / onet

Overlay Network for distributed protocols
GNU Lesser General Public License v3.0
51 stars 28 forks source link

Updates the ServerHTTP function to handle streaming from the client #616

Closed nkcr closed 4 years ago

nkcr commented 4 years ago

+ some refactoring

ineiti commented 4 years ago

I'm not sure it's not asking for problems the way you update tun in the go-routine... It seems to be racy to me.

Also, does it make sense to send commands once you opened a stream? The way you handle it now, if a command is sent, a new stream is created. But is the old one closed? It doesn't seem. So every command creates a new stream, correct?

If I'm correct that you're opening multiple streams at the same time, I have no idea how websocket handles it. My proposition is to either

But as I don't know what you're doing on the client side, I don't know if either of these make sense.

nkcr commented 4 years ago

I'm not sure it's not asking for problems the way you update tun in the go-routine... It seems to be racy to me.

Ok, I removed the update of the variable

Also, does it make sense to send commands once you opened a stream?

That allows the client to send additional message without needing to close the stream and re-open one, which is a feature of WebSocket.

The way you handle it now, if a command is sent, a new stream is created. But is the old one closed? It doesn't seem. So every command creates a new stream, correct?

No. In the way I implemented the service, the client gets back a streamID when it sends the first message. If it's the first message then the service creates a stream. Then, after the first message sent to the service, the client can re-send a message with the streamID, and the service will re-use the already created stream.

nkcr commented 4 years ago

@Gilthoniel thanks for the review, all points have been adressed, and tests for byzcoin/java pass.