2

I'm a fairly new developer and this one has me stumped.

My WinForms application is a slideshow for websites that rotates through a list of URLs, fading-in/out on each transition by using a second form as a "curtain". It's meant to run for an indefinite period of time but consistently hangs on the transition after running for a couple of days.

Form1:

HttpWebResponse response = null;
List<Slide.Doc> sList = null;

bool repeatSlideshow = true;
bool pageLoaded = false;

double curtainAnimStep = 0.05;
int errorCount = 0;

public Form1()
{
    InitializeComponent();

    CursorShown = false;

    this.Visible = true;
    this.FormBorderStyle = FormBorderStyle.None;
    this.WindowState = FormWindowState.Maximized;

    webBrowser1.ScrollBarsEnabled = false;
    webBrowser1.ScriptErrorsSuppressed = true;

    Slideshow(environment, channel);
}


public void Slideshow(string environment, string channel)
    {           
        while (repeatSlideshow)
        {
            try
            {
                sList = Slide.convertJSONToSlide(Slide.getParams(environment, channel));
            }
            catch (Exception)
            {
                Form2 curtain = new Form2(curtainAnimStep);
                curtain.Show();
                waitForFade(curtain, 1);
                displayError();
                raiseCurtain(curtain, curtainAnimStep);
                waitForFade(curtain, 0);
                curtain.Dispose();
                waitAround(30);
                continue;
            }

            foreach (Slide.Doc s in sList)
            { 
                bool slideWasDisplayed = false;

                Form2 curtain = new Form2(curtainAnimStep);
                curtain.Show();
                waitForFade(curtain, 1);
                slideWasDisplayed = displaySlide(s.URL_TEXT);
                if (slideWasDisplayed == false)
                {
                    webBrowser1.DocumentText = "<html><body style='background-color: #1C1C1C;'></body></html>";
                    redrawPage();
                }
                raiseCurtain(curtain, curtainAnimStep);
                waitForFade(curtain, 0);
                curtain.Dispose();
                if (slideWasDisplayed == true)
                {
                    waitAround(s.DISPLAY_SEC);
                }

            }

            if (errorCount == sList.Count)
            {
                Form2 curtain = new Form2(curtainAnimStep);
                curtain.Show();
                waitForFade(curtain, 1);
                displayError();
                raiseCurtain(curtain, curtainAnimStep);
                waitForFade(curtain, 0);
                curtain.Dispose();
                waitAround(30);                  
            }

            errorCount = 0;

            Utilities.Web.WebBrowserHelper.WebBrowserHelper.ClearCache();
        }
    }

    public bool displaySlide(string slideUrl)
    {
        HttpWebRequest request = (HttpWebRequest)WebRequest.Create(slideUrl);
        request.Timeout = 1000;
        try
        {
            response = (HttpWebResponse)request.GetResponse();
            webBrowser1.Navigate(slideUrl);
            redrawPage();
            response.Dispose();
            return true;
        }
        catch (WebException)
        {
            errorCount++;
            return false;
        }
    }

    public void redrawPage()
    {
        while (pageLoaded == false)
        {
            Application.DoEvents();
        }
        webBrowser1.Invalidate();
        Application.DoEvents();
        pageLoaded = false;
    }

    public void raiseCurtain(Form curtain, double curtainAnimStep)
    {
        while (curtain.Opacity > 0)
        {
            curtain.Opacity -= curtainAnimStep;
            Application.DoEvents();
            System.Threading.Thread.Sleep(10); // How long between shifts in opacity (NOT interval between slides)
        }
    }

    public void waitAround(int duration)
    {
        DateTime dt2 = DateTime.Now;
        while (dt2.AddSeconds(duration) > DateTime.Now)
        {
            Application.DoEvents();
        }
    }

    public void waitForFade(Form curtain, int finalOpacity)
    {
        while (curtain.Opacity != finalOpacity)
        {
            DateTime dt = DateTime.Now;
            dt = dt.AddSeconds(1);
            while (dt > DateTime.Now)
            {
                Application.DoEvents();
            }
        }
    }

    private void webBrowser1_DocumentCompleted(object sender, WebBrowserDocumentCompletedEventArgs e)
    {
        pageLoaded = true;
    }

Form2:

public Form2(double animStep)
        {
            InitializeComponent();
            this.AnimStep = animStep;
        }

        public double AnimStep { get; set; }

        private async void Form2_Load(object sender, EventArgs e)
        {
            while (Opacity < 1.0)
            {
                await Task.Delay(10);
                Opacity += AnimStep;
            }
            Opacity = 1;
        }

I've been working on this for a long time, but I have to admit that I genuinely don't even know what I should be looking for at this point.

Could the use of Application.DoEvents be responsible? Leaving them out breaks the application, but I can't figure out an alternative appproach.

Nitroid
  • 53
  • 1
  • 8
  • 1
    `DoEvents` on a tight loop is a code smell, it may cause unexpected reentrancy and probably is the source of your problems. See my comment [here](http://stackoverflow.com/questions/31408266/firing-webbrowser-documentcompleted-event-whilst-in-a-loop/31411201#comment50810454_31411201). – noseratio Jul 18 '15 at 00:37

1 Answers1

0

Looking at your code (and as indicated by Noseratio) one of the things I advice is to get rid of the need for the DoEvents calls. Just remember that in Windows there is a dedicated UI thread that is used to update the controls on the form. As you are doing a lot of stuff (in loops, calling a bunch of methods) on that same UI thread the Windows controls depends on your cooperation to share some time with them, hence the calls to DoEvents.

I'm going to use a BackgroundWorker and a Timer and WaitHandle to schedule commands that will update the UI from a background thread. With that we do as little as needed on the UI thread.

Form Load

Form1 will only have a webbrowsercontrol and a backgroundworker. A queue will hold the commands that needs to be executed. From the Load event we start the Backgroundworker.

    Form2 frm2 = new Form2();
    Queue<ICommandExecutor> commands = new Queue<ICommandExecutor>();

    private void Form1_Load(object sender, EventArgs e)
    {
        frm2.Show();
        frm2.BringToFront();
        commands.Enqueue(new LoadSlideShow(this, frm2, commands));
        backgroundWorker1.RunWorkerAsync();
    }

BackgroundWorker

The Backgroundworker DoWork event is the engine that runs on it's own background thread. It runs as long as there are commands found in the queue. After fetching a command it's Execute method is fired. If the command supports disposing the Dispose method is called and with that a command is processed and we start over again.

    private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
    {                
        while(commands.Count>0)
        {
            ICommandExecutor cmd = commands.Dequeue();
            try
            {
                cmd.Execute();
                // dispose if we can
                IDisposable sync = cmd as IDisposable;
                if (sync != null)
                {
                    sync.Dispose();
                }
            }
            catch(Exception exp)
            {
                // add commands here
                Trace.WriteLine("error" + exp.Message);
            }
        }   
    }

Commands

There is a standard interface available to implement a command pattern. ICommandExecutor has a single method, Execute. We can create different classes that implement this interface. Each class holds its own state and references and it can be as simple as a timer of as complex as loading a new batch of urls to show.

    public class ShowSlide:ICommandExecutor
    {
        string url;
        Form1 form;
        AutoResetEvent done = new AutoResetEvent(false);
        public ShowSlide(Form1 form, string url)
        {
            this.url = url;
            this.form = form;
        }

        public void Execute()
        {
            // if we are not on the UI thread...
            if (form.InvokeRequired)
            {
                // ... switch to it...
                form.Invoke(new MethodInvoker(Execute));
            }
            else
            {
                // .. we are on the UI thread now
                // reused from your code
                form.displaySlide(url);
            }
        }
    }

Here is a timer. Notice how a Timer class is used and the timerDone waithandle to make the backgroundthread continue work only if the timer has finished when Dispose is called.

    public class WaitForSeconds: ICommandExecutor, IDisposable
    {
        int ms;
        System.Threading.Timer timer;
        ManualResetEvent timerDone = new ManualResetEvent(false);
        public WaitForSeconds(int secs)
        {
            this.ms = secs * 1000;
        }

        public void Execute()
        {
            // use a timer
            timer = new System.Threading.Timer(
               (state) => timerDone.Set() // signal we are done
               );
            timerDone.Reset();
            timer.Change(this.ms, Timeout.Infinite);
        }

        public void Dispose()
        {
            timerDone.WaitOne();
            timerDone.Dispose();
            timer.Dispose();
        }
    }

To setup the commands in the correct order we use the following command class implememntation that takes the Command queue, Form1 and Form2 as parameters on its constructor. The Execute command loads all url's to be fed to the webbrowser control. For each url it adds the commands that needs to be executed to the queue. At the end the this instance is added to the queue as well which means the class will be used again if all commands have been processed. The queue will there for never be empty.

    public class LoadSlideShow: ICommandExecutor
    {
        readonly Queue<ICommandExecutor> commands;
        readonly Form1 form;
        readonly Form2 form2;
        public LoadSlideShow(Form1 form, Form2 form2, Queue<ICommandExecutor> cmds)
        {
            this.form = form;
            commands = cmds;
            this.form2 = form2;
        }

        public void Execute()
        {
            var list = Slide.convertJSONToSlide(null);
            foreach (var slide in list)
            {
                commands.Enqueue(new ShowSlide(form, slide.URL_TEXT));
                commands.Enqueue(new WaitForSeconds(1));
                //commands.Enqueue(new LowerCurtain(form2));
                commands.Enqueue(new WaitForSeconds(slide.DISPLAY_SEC));
                //commands.Enqueue(new RaiseCurtain(form2));
            }
            commands.Enqueue(this); 
        }
    }

This is basically all there is that is needed to get a basic slideshow going.

For the so called curtain we are going to do something similar with Form2 but I'll use the BackgroundWorker_progress event as well.

Form2 the Curtain

Form2 will act as the curtain by changing it's Opacity in a loop. It has it's own backgroundworker:

    ManualResetEvent stateChange = new ManualResetEvent(false);
    public ManualResetEvent stateChangeDone = new ManualResetEvent(false);

    private void Form2_Load(object sender, EventArgs e)
    {
        backgroundWorker1.RunWorkerAsync();  
    }

    private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
    {
        while(stateChange.WaitOne())
        {
            stateChange.Reset();
            var progressDone = new AutoResetEvent(false);
            int progress = 0;
            using(var timer = new System.Threading.Timer(_=>
                { 
                    backgroundWorker1.ReportProgress(progress);
                    progress += 2;
                    if (progress>=100)
                    {
                        progressDone.Set();
                    }
                }, null, 0, 25))
            {
                progressDone.WaitOne();
            }
            stateChangeDone.Set();
        }
    }

The background worker calls ResportProgress with an int indicating its prpgress. That causes the ProgressChanged event to be raised. Based on what state the Curtain needs to be in, we calculate the correct value for the Opacity.

    private void backgroundWorker1_ProgressChanged(object sender, ProgressChangedEventArgs e)
    {
        switch(state)
        {
            case Curtain.Up:
                this.Opacity = e.ProgressPercentage / 100.0;
                break;
            case Curtain.Down:
                this.Opacity = (100 - e.ProgressPercentage) / 100.0;
                break;
        }
    }

To get this all started we create two public methods called Up and Down:

    enum Curtain
    {
        Up,
        Down
    }

    Curtain state;

    public void Up()
    {
        state = Curtain.Up;
        stateChange.Set();
        stateChangeDone.Reset();
    }

    public void Down()
    {
        state = Curtain.Down;
        stateChange.Set();
        stateChangeDone.Reset();
    }

With that we are only left with the implementation of the Command classes that will be added to the Command queue and handled by the background worker of Form1:

    public class RaiseCurtain:ICommandExecutor, IDisposable
    {
        readonly Form2 form2;

        public RaiseCurtain( Form2 form2)
        {
            this.form2 = form2;
        }

        public void Execute()
        {
            if (form2.InvokeRequired)
            {
                form2.Invoke(new MethodInvoker(Execute));
            }
            else
            {
                form2.BringToFront();
                form2.Up();
            }
        }

        public void Dispose()
        {
            form2.stateChangeDone.WaitOne();
        }
    }

    public class LowerCurtain : ICommandExecutor,IDisposable
    {
        readonly Form2 form2;

        public LowerCurtain(Form2 form2)
        {
            this.form2 = form2;
        }

        public void Execute()
        {
            if (form2.InvokeRequired)
            {
                form2.Invoke(new MethodInvoker(Execute));
            }
            else
            {
                form2.Down();
            }
        }

        public void Dispose()
        {
            form2.stateChangeDone.WaitOne();
        }
    }

That is it. We have eliminated the use of DoEvents.

There is one caveat: this doesn't guarantee that the application will stop again after a couple of hours/days. The reason for this is a possible memory-leak in the webbrowser control and in my testing I did see the same effect, a slowly but steadily increasing private memory consumption while the managed memory bytes stayed virtually the same.

As none of the posts provided a definitive answer one option could be to restart your app as indicates in one of the answers here. On the plus side, you can implement this now as a Command class...

Community
  • 1
  • 1
rene
  • 41,474
  • 78
  • 114
  • 152