Secure Socket Async problem about fleck

[statianzo]

Could you give a sample to reproduce this error?

[smallwind1912]

hi I rewrite your code in sample application
Here is it

        static void Main()
        {
            FleckLog.Level = LogLevel.Debug;
            var allSockets = new List();
            var server = new WebSocketServer("wss://localhost:8181");
            server.Certificate = "MyCert.cer";
            server.Start(socket =>
                {
                    socket.OnOpen = () =>
                        {
                            Console.WriteLine("Open!");
                            allSockets.Add(socket);
                        };
                    socket.OnClose = () =>
                        {
                            Console.WriteLine("Close!");
                            allSockets.Remove(socket);
                        };
                    socket.OnMessage = message =>
                        {
                            Console.WriteLine(message);
                            
                            allSockets.ToList().ForEach(s => s.Send("Echo: " + message));
                        };
                });


            var input = Console.ReadLine();
            
            while (input != "exit")
            {
                foreach (var socket in allSockets.ToList())
                {
                    var t1 = new Thread(TestSendThread);
                    var t2 = new Thread(TestSendThread);
                    var t3 = new Thread(TestSendThread);
                    t1.Start(socket);
                    t2.Start(socket);
                    t3.Start(socket);
                }

                input = Console.ReadLine();
            }

        }

        public static void TestSendThread(object server)
        {
            var webSocketConnection = server as WebSocketConnection;
            if (webSocketConnection != null)
                webSocketConnection.Send("TestTEstTestTEstTestTEstTestTEstTestTEstTestTEstTestTEst");
        }
    }

and update client
window.ws = new wsImpl('wss://localhost:8181/consoleappsample', 'my-protocol');
I update MyCer.cer on console debug folder

When it send data to client exception to throw
Problem only happen in secure server .
Thank for your work

[statianzo]

Looks to be an issue with SslStream in .NET. Although, I haven't looked at the reflected source to be sure. In the mono SslStreamBase implementation, the stream will lock until the previous write operation is complete.

[yezhstudio]

Hi, I have an issue with Fleck.

In Fleck, when send data, you used Task. An Task will perform on a thread created by ThreadPool.
If you do many send in a very short time, I think ThreadPool will create many thread to do send task.

So, I don't know does all sent data will be same order with I call Send method.

[statianzo]

Send order is not guaranteed.

[yezhstudio]

I think send order for one connection sould be guaranteed.

[statianzo]

Feel free to send a pull request.

[yezh]

I am developing a online game. The client is running in browser. I use fleck in my server side, and I need keep a long connection for my client. It is very import keep the send order for a specified client connection.
I think fleck is very good, I do not want to fork a new Fleck.

[statianzo]

I'm not saying you'd need to maintain a fork, just fix your issue, create a pull request, and then I could pull it in to the Fleck codebase.

[statianzo]

Canan Kasaci wrote a wrapper for the SslStream that does support multiple BeginWrite calls. Once that is merged in, SSL woes on windows should be eliminated.

[JunkiXL]

Anything new about that fix? I'm really looking forward for this.

[PhanHoangAnh]

I have same problem, this prevent Fleck in secured sites, could you guy please pay more attention for this bug..??

[bshock]

We just saw this problem last week. Same situation and exceptions as originally posted. I'd really like to see this one fixed.

We briefly looked at Kasaci's "wrapper for the SslStream" that was mentioned earlier. (I wouldn't call it so much a wrapper for Sslstream as a wrapper for Stream.) There are a number of ways to fit this into SocketWrapper, but so far everything we've tried has caused Send to stop dead.

Looks like there hasn't been any movement on this bug for several months. Is this something that's being actively addressed, and if so, is there an ETA on the fix?

[statianzo]

It's not being actively addressed. I'd definitely bring in a pull request that fixes this though.

[bshock]

Someone online suggested to us that Kasaci's MyLockedStream class makes a poor assumption that reading and writing streams need to be blocked from each other. As I understand it, the NotSupportedException occurs because SslStream.BeginWrite doesn't work while there's a pending write, which means you really only need to block simultaneously writes (and maybe simultaneous reads) on the same stream. I apologize for not explaining this well.

The actual code suggestion is as follows:

1. Shoehorn the MyLockingStream class into Fleck's SocketWrapper.Authenticate

        public Task Authenticate(X509Certificate2 certificate, Action callback, Action error)
        {
            var ssl = new SslStream(_stream, false);
            var tempStream = new MyLockingStream(ssl);
            _stream = tempStream;
            //original:
            //var ssl = new SslStream(_stream, false);
            //_stream = ssl;
            Func begin =
                (cb, s) => ssl.BeginAuthenticateAsServer(certificate, false, SslProtocols.Tls, false, cb, s);
                
            Task task = Task.Factory.FromAsync(begin, ssl.EndAuthenticateAsServer, null);
            task.ContinueWith(t => callback(), TaskContinuationOptions.NotOnFaulted)
                .ContinueWith(t => error(t.Exception), TaskContinuationOptions.OnlyOnFaulted);
            task.ContinueWith(t => error(t.Exception), TaskContinuationOptions.OnlyOnFaulted);
            return task;
        }

2. Then in MyLockingStream, separate the async_event object into a separate object for reading and writing

        AutoResetEvent async_event_r;
        AutoResetEvent async_event_w;

3. Finally, place async_event_r and async_event_w in the appropriate sections of MyLockingStream for reading and writing

        public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback callback, object state)
        {
            if (!CanRead)
                throw new NotSupportedException("This stream does not support reading");
            if (async_event_r == null)
            {
                lock (this)
                {
                    if (async_event_r == null)
                        async_event_r = new AutoResetEvent(true);
                }
            }
            async_event_r.WaitOne();
            return _inner.BeginRead(buffer, offset, count, callback, state);
        }
        public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback callback, object state)
        {
            //Create async_event if null
            if (async_event_w == null)
            {
                lock (this)
                {
                    if (async_event_w == null)
                        async_event_w = new AutoResetEvent(true);
                }
            }
            async_event_w.WaitOne(); //Only allow one thread through
            return _inner.BeginWrite(buffer, offset, count, callback, state);
        }
        public override void EndWrite(IAsyncResult result)
        {
            try
            {
                _inner.EndWrite(result);
            }
            finally
            {
                async_event_w.Set(); //Signal so the next thread can pass through
            }
        }
        public override int EndRead(IAsyncResult result)
        {
            try
            {
                return _inner.EndRead(result);
            }
            finally
            {
                async_event_r.Set(); //Signal so the next thread can pass through
            }
        }

If we just include MyLockingStream in its original form, SSL writes end up blocked by async_event.WaitOne().

With the simple modifications, however, Fleck seems to work correctly with SSL WebSockets in my company's custom server.

We also ran these tests with smallwind1912's test server from the beginning of this thread. The problems and the solution seemed to work exactly the same way as we'd seen with our server.

We've only been testing this solution for a few hours and in the relatively limited uses our server gives it. There may be a lot of side effects we haven't considered yet. But for the moment it seems to fix this bug.

[orcun]

First of all, I feel sorry not contributing before. I have previously solved this issue using a queue in socket wrapper. I have to admit, not the right place but it was working and simple, thanks to Task model. I was going to suggest that but wrapping sslstream gave me an idea. So I wrote a stream wrapper with queued writes. The advantage is that, writes are not blocked. At least, blocking is not an option for me.

Idea is that, write callbacks are wrapped so that queued items are processed.

I have to note that my first attempt, linked above, was a failure. Turns out AsyncResult model is more complicated :) Now that I have added a unit test, I have solved the remaining issues. So please take a look at my last commit.

I would be more than happy to send a pull request but looking at network graph, there seems to be strange issue. Should I try to fix that first and any ideas? BTW, there is a secure branch, just related to this issue which I guess I should publish as it is not visible, now that I have checked.

[statianzo]

@orcun Thanks for the contribution. I'll dig into it soon. If you don't hear anything back in a week, please nag me.

[orcun]

Great news! btw, I have updated a prod server a few hours ago (thousands of daily active users) and so far everything looks fine.

[jonagh]

Just wondering if this fix will be pulled in soon? Thank you kindly.

[statianzo]

@jonagh If you want it brought in, make a pull request of it.

[StevenBonePgh]

@rickparrish @jonagh Can either of you please submit a pull request for your fixes on this issue, assuming they are stable?

[jonagh]

Sorry, I didn't make the fixes, I just pulled them into my fork from orcun.
(BTW, I apologize for all the issue references I made above, cluttering the thread.)

[ugs]

i submitted pull request #101 containing the fix from @jonagh.