0

I'm using the ZedGraph library, but on a website rather than Web Forms application. Quick intro - the app gets data from a database, generates a graph, saves it as an image and then displays the image on a web page.

I have a GraphController class, which basically looks like this:

public class GraphController {

    private static GraphController instance;
    private static ZedGraph.ZedGraphControl graph;
    protected GraphController() { }

    public static GraphController Instance {
        get {
            if (instance == null){
                instance = new GraphController();
            }
            return instance;
        }
    }

    public string GenerateGraph(Dictionary<DateTime, float> graphData, DataRow details, string graphType) {
        if ((graph == null) || (graph.IsDisposed == false)) {
            graph = new ZedGraph.ZedGraphControl();
        }
        graph.GraphPane.Title.IsVisible = false;
        // function calls to generate graph. Graph object is referenced in these.
        return SaveGraphAsImage(0000, graphType);
    }

    private void AddGridLines() {
        // example of graph reference
        graph.GraphPane.XAxis.MajorGrid.IsVisible = true;
    }

    private string SaveGraphAsImage(string paramId, string graphType) {
        // Should deal with save location here 
        string location = HttpRuntime.AppDomainAppPath + "Graphs\\" + DateTime.Now.ToString("dd-MM-yyyy");
        string filename = DateTime.Now.ToString("dd-MM-yyyy-HH-mm-ss-") + graphType + "-" + paramId + ".png";

        if(System.IO.Directory.Exists(location) == false){
            System.IO.Directory.CreateDirectory(location);
        }
        graph.Refresh();

        try {
            using (graph) {
                string outputFileName = location + "\\" + filename;

                if (File.Exists(outputFileName) == false) {
                    Bitmap image = graph.GraphPane.GetImage(960, 660, 180);
                    image.Save(outputFileName, ImageFormat.Png);
                    image.Dispose();
                }
            }
            graph = null; // double check that it is removed.
            return "../Graphs/" + DateTime.Now.ToString("dd-MM-yyyy") + "/" + filename;
        }
        catch (Exception ex) {
            log("An error occured generating a graph. \n\n Error message: \n " + ex.Message + " \n\n Stack trace: \n " + ex.StackTrace);
            graph = null;
            return "#";
        }
    }
}

I've reduced it as much as possible, but it should show how the ZedGraph object is created, used and removed.

I've already tried some memory enhancements, getting rid of the graph object where possible, and trying the using(){} process to hopefully automatically remove it, but would welcome further suggestions.

My aim here is to improve memory management and reduce these errors. Running local performance tests, I get quite a few Control 'ZedGraphControl' accessed from a thread other than the thread it was created on. errors. I'm relatively new to threading, so I'm not sure if a) it is something that is needed here, or b) there's something that can be done to disable the possibility, or a better management of the graph object to ensure it is always within the same thread?

Edit: This GraphController is first called from an .aspx.cs page with the following: GraphController.GraphController.Instance.GenerateGraph(data, details, "graph-type");

Second Edit: I have re-written the code to not have the ZedGraphControl as private static, but instead it is created within the GenerateGraph and then passed around where necessary. Testing this up to 1,000 users in 60 seconds looks to have removed the cross-thread issues - does this sound likely?

edparry
  • 688
  • 1
  • 10
  • 34
  • does this sound likely? - Yes, and it shows you why to avoid `static` in Web apps. – H H Nov 10 '15 at 12:07
  • Thanks @HenkHolterman - sounds like something I need to read up on then! Also - would you recommend changing/not using the Instance process in this scenario as well? – edparry Nov 10 '15 at 12:11
  • 1
    "recommend not using the Instance process" Yes, it looks harmless but there is no gain either. Keep it simple, keep scopes and lifetimes as small as possible. – H H Nov 10 '15 at 13:07

1 Answers1

0

The issue here was the use of static variables. As pointed out by Henk Holterman in the comments, static variables stay alive as long as the application process is alive, and is separate from any/all user requests. So by having the ZedGraph object as a static variable it mean't that it was potentially still available to multiple requests without being cleared properly, running into memory and cross-threading issues.

The solution is to declare the ZedGraph instance within the first GenerateGraph function and to then pass the same object to each other function that used it. This ensures that it is the same object in the same thread that is accessed throughout the process.

edparry
  • 688
  • 1
  • 10
  • 34