jgauffin / Griffin.WebServer

A web server built on top of Griffin.Framework
107 stars 42 forks source link

HTTPS port will stop responding after some time. #33

Closed henon closed 7 years ago

henon commented 7 years ago

HTTP port works always. But when I open two ports, one HTTP, one HTTPS or just an HTTPS port the server will stop responding after some time on the HTTPS port while still responding on the HTTP port. Any ideas?

henon commented 7 years ago

Found the reason! It is a channel / bufferslice leak. SecureTcpChannel's exception handling is not as well as TcpChannel's, so it happens that the channels get less and less which allocates more and more buffer slices until they are all spent. By the way, buffers are never ever returned to the buffer slice pool ever.

jgauffin commented 7 years ago

awesome! will look at it tonight.

henon commented 7 years ago

That would be great. I attempted a fix that I don't know yet if it really helps so I would love to see your solution because I don't think I completely understand the code in its whole complexity yet.

Anyway, just let me say that when the https port hangs new requests come in at OnAcceptSocket#OnAcceptSocket.

_channels is empty (as is _usedChannels) so a new channel is created which throws exception because the _bufferPool is out of buffer slices.

henon commented 7 years ago

Looks like you are really too busy to check that out. I can confirm now, that my fix works well. Basically, channels get lost if they are popped from _channels and then an exception is thrown. I just call OnChannelDisconnect in the catch block. Here it is: replace ChannelTcpListener#OnAcceptSocket with this:

    private void OnAcceptSocket(IAsyncResult ar)
    {
        if (_shuttingDown)
            return;

        ITcpChannel channel=null;
        try
        {
            // Required as _listener.Stop() disposes the underlying socket
            // thus causing an objectDisposedException here.
            var socket = _listener.EndAcceptSocket(ar);

            if (!_channels.TryPop(out channel))
            {
                var decoder = _configuration.DecoderFactory();
                var encoder = _configuration.EncoderFactory();
                channel = _channelFactory.Create(_bufferPool.Pop(), encoder, decoder);
            }

            channel.Disconnected = OnChannelDisconnect;
            channel.MessageReceived = OnMessage;
            channel.Assign(socket);

            _usedChannels.TryAdd(channel.ChannelId, channel);

            var args = OnClientConnected(channel);
            if (!args.MayConnect)
            {
                if (args.Response != null)
                    channel.Send(args.Response);
                channel.Close();
                return;
            }
        }
        catch (Exception exception)
        {
            if (channel != null)
            {
                OnChannelDisconnect(channel, exception);
            }
            OnListenerError(exception);
        }

        _listener.BeginAcceptSocket(OnAcceptSocket, null);
    }
henon commented 7 years ago

I now identified the root cause for the blocking of the HTTPS port. It would hang in ServerSideSSLStreamBuilder.Build for certain requests (seems to be waiting for client response that never comes).

I changed the one line from indefinitely blocking call

stream.AuthenticateAsServer(Certificate, UseClientCertificate, Protocols, CheckCertificateRevocation):

to allow failing within a short time period:

stream.AuthenticateAsServerAsync(Certificate, UseClientCertificate, Protocols, CheckCertificateRevocation).Wait(TimeSpan.FromSeconds(3));

Since this change, the server still seems to hang for a short time when such a client connects but it will return to normal opertation and hasn't gone into full blocking ever since I made the change.

DanyD commented 7 years ago

Hi! Is there any chance to get this fix commited in the near future?

jgauffin commented 7 years ago

yes, sorry. Will fix it this week.

DanyD commented 7 years ago

Hi, didn't it work?

henon commented 7 years ago

DanyD, if you are intereseted in my fixes I can push them back to my fork here on github

jgauffin commented 7 years ago

again, apologies for the delay. I just published a new Griffin.Framework commit which includes the fix. Så update it in your webserver project.

DanyD commented 7 years ago

Hi Gauffin, thank you. I just recompiled and tested, but now we get an error when trying to start the server with a certificate file:

ERROR: System.MissingMethodException: Method not found: "Void Griffin.Net.Channels.ServerSideSslStreamBuilder..ctor(System.Security.Cryptography.X509Certificates.X509Certificate)". bei Griffin.WebServer.HttpServer.Start(IPAddress ipAddress, Int32 port, X509Certificate certifiate)

jgauffin commented 7 years ago

ok. I'll publish a new webserver package.

DanyD commented 7 years ago

Can I download the new package somewhere as it is not yet available at nuget?