Sending screenshots taking too much memory

Question

I am trying to make a small application to serve screenshot of entire screen through network. I want it to be able to serve one every 1-2 seconds through LAN. I thought it won't be a big problem, so I chose C# and nancy www self host (as the easiest option). Now, the speed is allright for my needs, but it seems to be taking way too much memory, and it grows with time. It seems to settle on taking about 3.5 GB RAM and from then no longer grows, but that's not an acceptable amount - even with big resolution (mine is 2560x1440).

Is my code bad, or is nancy not suitable for handling many big responses, or maybe C# method of capturing screen is poorly optimized and I should try pinvoke methods? Or maybe it's just a terrible way to do it and I should try something more advanced, like using VNC library?

Currently my code looks like this:

public class HomeModule : NancyModule
{
    private Bitmap bitmapScreenCapture;
    private Graphics graphics;
    private Object lockMe = new object();
    private MemoryStream memoryStream = new MemoryStream();

    public HomeModule()
    {
        Get["image"] = parameters =>
        {
            lock (lockMe)
            {
                GC.Collect();

                if (bitmapScreenCapture == null || bitmapScreenCapture.Width != Screen.PrimaryScreen.Bounds.Width || bitmapScreenCapture.Height != Screen.PrimaryScreen.Bounds.Height)
                {
                    if (bitmapScreenCapture != null)
                    {
                        bitmapScreenCapture.Dispose();
                    }
                    bitmapScreenCapture = new Bitmap(Screen.PrimaryScreen.Bounds.Width, Screen.PrimaryScreen.Bounds.Height);
                    graphics = Graphics.FromImage(bitmapScreenCapture);
                }

                graphics.CopyFromScreen(Screen.PrimaryScreen.Bounds.X,
                    Screen.PrimaryScreen.Bounds.Y,
                    0, 0,
                    bitmapScreenCapture.Size,
                    CopyPixelOperation.SourceCopy);

                bitmapScreenCapture.Save(memoryStream, ImageFormat.Png);
                memoryStream.Position = 0;

                return new Response()
                {
                    Contents = stream => memoryStream.CopyTo(stream)
                };
            }
        };
    }
}

Answer 1

As much as possible, keep variables in the most local scope possible, and dispose of what you can.

Part of your issue may be that you're new'ing up a Graphics instance repeatedly, but never disposing of the old reference. The GC will collect it eventually, but you can place your code in a using block to let it know you're done with it.

I haven't tested this, but here I've made your instances local, and disposed of the Graphics and Bitmap instances. I didn't dispose of the MemoryStream since I'm not sure it will successfully return the value if you do, but you could play around with it.

var screen = Screen.PrimaryScreen;

using (var bitmap = new Bitmap(screen.Bounds.Width, screen.Bounds.Height))
{
    using (var g = Graphics.FromImage(bitmap))
    {
        g.CopyFromScreen(screen.Bounds.Left, screen.Bounds.Top, 0, 0, screen.Bounds.Size);
    }

    var imageStream = new MemoryStream();
    bitmap.Save(imageStream, ImageFormat.Png);
    imageStream.Position = 0;

    return new Response()
    {
        Contents = stream => memoryStream.CopyTo(stream)
    };
}