Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Empty buffer when null buffer #439

Merged
merged 2 commits into from
Dec 3, 2015
Merged

Empty buffer when null buffer #439

merged 2 commits into from
Dec 3, 2015

Conversation

benaadams
Copy link
Contributor

Resolves #438

dbug: Microsoft.AspNet.Server.Kestrel[1]
      Connection id "1" started.
info: Microsoft.AspNet.Hosting.Internal.HostingEngine[1]
      Request starting HTTP/1.1 GET https://space.ageofascent.com/?&name=test-Ssl
dbug: Microsoft.AspNet.Server.Kestrel[6]
      Connection id "1" received FIN.
info: Microsoft.AspNet.Hosting.Internal.HostingEngine[2]
      Request finished in 340969ms 101
dbug: Microsoft.AspNet.Server.Kestrel[7]
      Connection id "1" sending FIN.
dbug: Microsoft.AspNet.Server.Kestrel[10]
      Connection id "1" disconnected.
dbug: Microsoft.AspNet.Server.Kestrel[8]
      Connection id "1" sent FIN with status "0".
dbug: Microsoft.AspNet.Server.Kestrel[2]
      Connection id "1" stopped.

vs the Exception thrown in #438; Websocket comms then works fine over Ssl; tested for 5mins as shown above Request finished in 340969ms 101

@benaadams
Copy link
Contributor Author

@benaadams
Copy link
Contributor Author

Currently testing 1.2M application messages on the websockets per second; tested 1,070,637,277 messages so far, seems happy; will leave it running overnight.

@benaadams
Copy link
Contributor Author

Fell over ticking into 10 billion messages; but that was an overflow on how I was recording, otherwise seems good.

@benaadams
Copy link
Contributor Author

Hmm, over 10Bn messages on a restart - maybe I shouldn't have tweeted the url... :P

@jonburney
Copy link

Nothing like Twitter for free load testing!

@halter73
Copy link
Member

halter73 commented Dec 1, 2015

Can we get a regression test?

@benaadams
Copy link
Contributor Author

Can we get a regression test?

Can do a filter that's unhappy with null buffers? I have a love hate relationship with ssl...

@halter73
Copy link
Member

halter73 commented Dec 1, 2015

Whatever works. Maybe you can unit test StreamSocketOutput with an outputStream that throws on null arrays.

@benaadams
Copy link
Contributor Author

Was having trouble recreating the error state in a test; but narrowed down; its from a combination of WebSocketMiddleware.UpgradeHandshake and HttpsConnectionFilter/SslStream at the same time which is why existing tests didn't trigger it.

101Bn messages tested so far; some issues; need to look into if its NPC load or WebSocketMiddleware - but working mostly - error rate is low for that number of messages :)

@benaadams
Copy link
Contributor Author

@halter73 can I pull in the WebSocketMiddeware for the test; and do a proper SSL+WebSocket test? (Build process-wise)

@halter73
Copy link
Member

halter73 commented Dec 2, 2015

@benaadams I think that would be fine, but may be more complicated than what is necessary.

I was suggesting something simple like:

var streamSO = StreamSocketOutput(throwsOnNullWriteStream, null);
streamSO.Write(default(ArraySegment<byte>), true); // this shouldn't throw

Of course, if you want to add a functional test too, I'm not going to stop you!

@Tratcher
Copy link
Member

Tratcher commented Dec 2, 2015

@benaadams No, this repo can't depend on the WebSocket repo. You could write the test in the WebSocket repo though.

@benaadams
Copy link
Contributor Author

@halter73 did as you suggested.

Test fails without the change; passes with it.

@halter73
Copy link
Member

halter73 commented Dec 2, 2015

Awesome. I'll merge this today.

@benaadams
Copy link
Contributor Author

Giving AppVeyor a force push (rebase)

@halter73 halter73 merged commit a85f376 into aspnet:dev Dec 3, 2015
@benaadams benaadams deleted the ssl branch December 3, 2015 08:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants