What is the best way to stop developers from calling System.Windows.Forms.Application.DoEvents() ?

Question

We just spent 300 man-hours fixing a buggy application in the field. It all came down to calling Application.DoEvents (re-entrancy problem).

This was not caught in design reviews, code reviews. The code was inserted two years ago with the first version; the application was always "flaky" but recent changes exposed the re-entrancy problems to a greater degree.

This incident is the second time in our organization that Application.DoEvents caused failures and multi-man hours of debugging. It was discovered in this case by simply noticing the call, buried way down in a complex event handler for an asynchronous task.

What do you suggest to prevent this issue from happening again:

• Add checkin gates to source control?
• Developer training?
• Code Analysis rules (why is this not already a built-in rule?)

How to I enforce a coding practice?

Answer 1

All of the above. You need to teach people the rules before you can expect any chance of success. You may also want to tell people, why the rules are important.

Following that tools that prevent you from breaking the rules are always a good idea. Your specific problem could be addresses by a FxCop rule or even a check-in policy.

Related question: http://stackoverflow.com/questions/377218/do-you-have-coding-standards-if-so-how-is-it-being-enforced

Answer 2

Every time the application is built centrally, run this on every assembly:

ildasm MyAssembly.exe /TEXT

Then search the output for:

System.Windows.Forms.Application::DoEvents

If it's found, mark the build as failed, as if it was a compile error.

Answer 3

I would suggest some training on asynchronous programming (using BeginInvoke) and doing time consuming tasks in the background on another thread.

Answer 4

Maintain a Development standards and add this to it.

Answer 5

A more effective way to prevent this is to write an FxCop rule which flags usages of this API. If FxCop is enabled as part of your build process this will eliminate it at the earliest possible time, build

Answer 6

"We just spent 300 man-hours fixing a buggy application in the field. It all came down to calling Application.DoEvents (re-entrancy problem)."

First let me say that I am not advocating the use of Application.DoEvents for casual use, but write a for x as long = 0 to long.maxvalue loop without one and see how responsive the UI is.

Without knowing what happened exactly it is hard to say whether it is really the issue or not.

Private Sub foo()
    stpw.Reset() : stpw.Start()
    Do
    Loop While stpw.ElapsedMilliseconds < 1000
    stpw.Stop()
    Debug.WriteLine("foo")
End Sub
Dim stpw As New Stopwatch
Private Sub Button3_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button3.Click
    Debug.WriteLine("")
    Debug.WriteLine("but click")
    Dim t As New Threading.Thread(AddressOf foo)
    t.Start()
    Do
        Threading.Thread.Sleep(10)
        'Application.DoEvents() 'uncomment to change the behavior
    Loop While stpw.IsRunning
    Debug.WriteLine("but exit")
End Sub

So lets say I originally wrote it without the offending statement, but the user was complaining about the UI being unresponsive. So I then add DoEvents but I now get odd results when users double-click the button. Is the problem DoEvents or just bad design. I'd vote for bad design in this case.

Answer 7

Would the absence of DoEvents make this "good" code?

Private Sub Button4_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button4.Click
    'simulate a long running task
    For x As Long = 1 To Long.MaxValue - 1
        'the absence of Application.DoEvents() is poor design IMHO
    Next
End Sub

Answer 8

Besides trying to prevent the call from being used there is a way to make it more likely to catch a problem like this (and other problems as well):

Use a lot of ASSERTs in your code.

I had a similar problem in my application (MSQuant) with DoEvents() and re-entrancy. But it was catched early by an ASSERT during my own testing and never affected any user.

ASSERTs catch errors early and I have saved many, many, many hours of debugging time. Plus they may catch bugs that otherwise go unnoticed (e.g. producing incorrect results).

It is a very efficient practice (high reward to effort ratio). I learned about it many years ago and have used it ever since. And of course it does not replace other practices: sound software engineering, unit testing, code standards, acceptance testing, code inspection.

Note that firing ASSERTs may not necessarily result in ASSERT dialogs (stopping normal execution or stopping the user from working) or aborting the program. It can be configured to send the information to a logging system (e.g. to a file), over a network, etc.