Form Management: Tight Or Incorrect Coupling Can Cause Hard To Find Bugs

   1: public class LaunchForm: Form

   2: {

   3:     //a bunch of other form/view related stuff

   4:  

   5:     private void Login()

   6:     {

   7:         LoginForm loginForm = new LoginForm();

   8:         loginForm.Owner = this;

   9:         DialogResult result = loginForm.ShowDialog();

  10:         DialogResult = result;

  11:     }

  12: }

  13:  

  14: public class LoginForm: Form

  15: {

  16:     //a bunch of other form/view related stuff

  17:     

  18:     private void Completed()

  19:     {

  20:         DialogResult = DialogResult.OK;

  21:         ShowRolesForm form = new ShowRolesForm();

  22:         form.ShowDialog();

  23:     }

  24: }

</div> </div>

Can you spot the bug? No, it’s not some style issue or architectural best practice… there is an application-crashing bug in this code.

A Memory Leak And An Exception

According to the MSDN documentation, setting the DialogResult of a dialog modal window will cause the ShowDialog call to return the value but will not cause the form to close. Instead, it just hides the form. Since the LoginForm is immediately calling out to the ShowRolesForm form after setting the DialogResult, the LoginForm is still held in memory because the ShowRolesForm form is blocking the LoginForm’s thread and it can’t exit.

This causes two problems:

1. Memory Leak: Until ShowRolesForm closes or returns a DialogResult, LoginForm is held in memory… if ShowRolesForm is the main window that the user interacts with, you essentially have a memory leak with the LoginForm still being around
2. Exception: ShowDialogForm still returns it’s result immediately when LoginForm set it’s own DialogResult value. When LaunchForm receives that value it tries to set it’s own DialogResult value. But since LoginForm is still alive and is still parented by LaunchForm, LaunchForm is not allowed to set a dialog result yet – the runtime thinks that LoginForm is still the top most dialog stack which causes LaunchForm to throw an exception when it tries to set it’s DialogResult, crashing the app.

Decouple Your Workflow From Your Forms

The solution to this situation is fairly straightforward: decouple the workflow of your system from the forms in your system. I’ve talked about this process a number of times on my blog and you can read all my posts on the Application Controller for information on how to go about doing this from a larger perspective.

In this particular case, a simple object to control the flow between LaunchForm, LoginForm and ShowRolesForm will do what we need. Each form should make sure to .Close() itself before setting the DialogResult.

   1: public class SomeWorkflowService

   2: {

   3:     public void Run()

   4:     {

   5:         var launchForm = new LaunchForm();

   6:         var launchResult = launchForm.ShowDialog();

   7:         launchForm.Dispose();

   8:         

   9:         //do something to check the launchResult here and determine

  10:         //if we need to go on

  11:         

  12:         var loginForm = new LoginForm();

  13:         var loginResult = loginForm.ShowDialog();

  14:         loginForm.Dispose();

  15:         

  16:         //do something to check the loginResult here and determine

  17:         //if we need to go on

  18:         

  19:         var showRolesForm = new ShowRolesForm();

  20:         var showRolesResult = showRolesForm.ShowDialog();

  21:         showRolesForm.Dispose();

  22:         

  23:         //do something with the showRolesResult here

  24:     }

  25: }