jacksonh / manos

Manos is an easy to use, easy to test, high performance web application framework that stays out of your way and makes your life ridiculously simple.
Other
455 stars 61 forks source link

Problem with async writing to response (using Boundary) #102

Closed konrad-kruczynski closed 13 years ago

konrad-kruczynski commented 13 years ago

Let's assume I have a project with file downloading. Files are contained on other server and can be downloaded from it using own protocol via sockets. First let's see handler:

        private void GetFile(IManosContext ctx, string pathAndFileName)
        {
            SessionObject session;
            if (CheckAndGetSession(ctx, out session))
            {
                Boundary.Instance.ExecuteOnTargetLoop(() => PerformDownload(ctx, session, pathAndFileName));
            }
        }

The PerformDownload method is quite long (because it deals with mentioned protocol), but it writes to response chunks of data using ctx.Response.Write(array of bytes here) and finishing it with ctx.Response.End() (after all chunks are written). If there are a lot of data to write, response seen by web browser is corrupted (seems to be truncated) and almost always in the same way. Protocol is quite well tested, I also have another (local app) client which works fine with many asynchronous downloads. Also arrays of bytes given to ctx.Response.Write() are never reused, so this is probably not corruption of data buffer before writing.

I also used a version with Boundary.Instance.ExecuteOnTargetLoop called on every write and end (and with method called directly) with no more luck (however I consider such approach more proper, changed it only to see if it helps).

Is it proper way to do that?

konrad-kruczynski commented 13 years ago

Oh, and when I am sending all data using just ctx.Response.End(data here), everything is fine.

konrad-kruczynski commented 13 years ago

Just made test case:

        [Route("/test")]
        public void Test(IManosContext ctx)
        {
            ThreadPool.QueueUserWorkItem(delegate
                                             {
                                                 Async(ctx);
                                             });
        }

        public void Async(IManosContext ctx)
        {
            for(var i = 0; i < 1000; i++)
            {
                Thread.Sleep(10);
                var now = i;
                Boundary.Instance.ExecuteOnTargetLoop(() => ctx.Response.Write(now + "Test line\n"));
            }
            Boundary.Instance.ExecuteOnTargetLoop(() => ctx.Response.End());
        }

which works fine. So this is probably some issue with my application. Sorry bothering you, I'm closing issue.

konrad-kruczynski commented 13 years ago

OK, I just discovered that this happens with big chunks of data. When chunks are limitated to 10kB, everything works fine. In other words: problems are with ctx.Response.Write(array) when array is quite big (above 100kB probably).

konrad-kruczynski commented 13 years ago

Here's updated test case which gives me corrupted response:

        [Route("/test")]
        public void Test(IManosContext ctx)
        {
            ThreadPool.QueueUserWorkItem(delegate
                                             {
                                                 Async(ctx);
                                             });
        }

        public void Async(IManosContext ctx)
        {
            var array = new byte[100*1024];
            for (var i = 0; i < array.Length; i++ )
            {
                array[i] = (byte)'a';
            }
            for (var i = 0; i < 10; i++)
            {
                Thread.Sleep(10);
                var now = i;
                Boundary.Instance.ExecuteOnTargetLoop(() => ctx.Response.WriteLine(now.ToString()));
                Boundary.Instance.ExecuteOnTargetLoop(() => ctx.Response.Write(array));
                Boundary.Instance.ExecuteOnTargetLoop(() => ctx.Response.WriteLine(string.Empty));
            }
            Boundary.Instance.ExecuteOnTargetLoop(() => ctx.Response.End());
        }
ghost commented 13 years ago

Thank you, should be fixed now.

jacksonh commented 13 years ago

This is probably the wrong way of doing things. You should be using the evented IO not a thread.

Something like:

var download = new HttpRequest (...); download.OnData += delegate (byte [] data) { response.Write (data); }; download.Complete += { response.End (); };

konrad-kruczynski commented 13 years ago

Thanks. It would be ideal to have some ,,best practices in Manos'' documentation (or more examples). That means I should make my own HttpRequest-like class to deal with my protocol, am I right?

jacksonh commented 13 years ago

Oh, is the download via some other protocol? In that case yeah, you should create something like HttpRequest for that protocol using an evented socket stream.

Basically anything involving a thread is a "worst case practice" in Manos ;)

On Mon, Jun 27, 2011 at 2:20 PM, konrad-kruczynski reply@reply.github.com wrote:

Thanks. It would be ideal to have some ,,best practices in Manos'' documentation (or more examples). That means I should make my own HttpRequest-like class to deal with my protocol, am I right?

Reply to this email directly or view it on GitHub: https://github.com/jacksonh/manos/issues/102#issuecomment-1449353

konrad-kruczynski commented 13 years ago

Fine. Also one question about the session - currently it is made using ConcurrentDictionary using GUIDs as keys (which are also put in cookies). Is it right approach? Does Manos have some kind of timer which can be used to invalidate/remove session? (Asking as I did not found any built-in session mechanism, maybe there is one).

konrad-kruczynski commented 13 years ago

I just found Timeout class, looks promising.