0

I have created a simple window form(pasted below). Now I use this to display a wait dailog to the user when running a complex operation which consumes time.

Calling the HerculesWaitForm.Show("Some Title","Some Caption...) shows the form as usual but when calling HerculesWaitForm.Close still shows the form. What can I do to overcome the problem.

Imports System.Threading
Public Class HerculesWaitForm
    Inherits DevExpress.Utils.WaitDialogForm
    Private Shared form As HerculesWaitForm
    Private Shared _thread As Thread
    Private Shared _caption As String, _title As String
    Private Sub New(ByVal caption As String, ByVal title As String)
        MyBase.New(caption, title)
    End Sub
    Private Shared Sub CreateInstance()
        form = New HerculesWaitForm(_caption, _title)
        Application.Run(form)
        Application.DoEvents()
    End Sub
    Public Overloads Shared Sub Show(ByVal caption As String, ByVal title As String)
        _caption = caption
        _title = title
        If form Is Nothing Then
            _thread = New Thread(AddressOf CreateInstance)
            _thread.IsBackground = True
            _thread.Start()
        End If
    End Sub
    Public Overloads Shared Sub SetCaption(ByVal caption As String)
        If form IsNot Nothing Then
            _caption = caption
            form.SetFormCaption()
        Else
            Show(caption, "")
        End If
    End Sub
    Public Overloads Shared Sub Close()
        If form IsNot Nothing Then
            form.CloseForm()
            form = Nothing
        End If
    End Sub
    Private Sub CloseForm()
        If Me.InvokeRequired Then
            Invoke(New MethodInvoker(AddressOf CloseForm))
            Return
        End If
        Application.ExitThread()
    End Sub
    Private Sub SetFormCaption()
        If Me.InvokeRequired Then
            Invoke(New MethodInvoker(AddressOf SetFormCaption))
            Return
        End If
        MyBase.SetCaption(_caption)
    End Sub
End Class
Soham Dasgupta
  • 5,061
  • 24
  • 79
  • 125
  • Do you get any exceptions on this? Check out this question, anyway: http://stackoverflow.com/questions/947476/calling-a-windows-form-from-another-thread-net – default locale Mar 13 '13 at 09:32
  • If the operation is completing very fast then the problem persists but if the it takes some time then it working as expected. – Soham Dasgupta Mar 13 '13 at 09:43
  • Bunch of code, doesn't actually have a Form.Close() call. HerculesWaitForm is a *type name*, never use HerculesWaitForm.Close(). That uses the wrong object when you do that in a thread. – Hans Passant Mar 13 '13 at 12:36
  • @HansPassant - I thought that too, but `HerculesWaitForm.Close()` actually calls the shared, overloaded `Close()` function that correctly closes the form (so long as it has been created by then). The whole class is a brutal mess of a way to implement a singleton, and it seems to actually "work", at least so well as it is intended. I call the whole thing foul, in any case. It manages to tick as many bad practice boxes as possible... – J... Mar 13 '13 at 13:43

2 Answers2

2

If you are calling this as :

Private Sub doSomethingLong()
    HerculesWaitForm.Show("hi", "there")
    Sleep(someAmount)  'Random long operation '
    HerculesWaitForm.Close()
End Sub

Then you can have problems, as you note, if the time between open and close calls is short. This is because the call to Show() starts a second thread working at creating the new hercules object while you are performing your operation. The long operation begins immediately and at the same time as the thread starts on the task of getting itself up and running.

If the call to Close() comes before the thread has finished initializing itself and completing the instantiation of the form object then the call to Close() will find that form = Nothing and it will simply return without doing anything.

Changing, then, the Show() code like this :

Public Overloads Shared Sub Show(ByVal caption As String, ByVal title As String)
    _caption = caption
    _title = title
    If form Is Nothing Then
        _thread = New Thread(AddressOf CreateInstance)
        _thread.SetApartmentState(ApartmentState.STA)  'should do this '
        _thread.IsBackground = True
        _thread.Start()
    End If
    While form Is Nothing   ' add   '
        Thread.Sleep(1)     ' this  '
    End While               ' here  '
End Sub

Will force the Show() method to block until the worker thread has created the form object - this ensures that all future calls to Close will not simply be skipped (because form is nothing). Not pretty, but without refactoring this whole class, it's probably the quickest fix.

This is actually a really bad way of dealing with a long operation, however. Rather than putting the work into a worker thread, you are creating a NEW UI thread to run a silly animation while blocking the real UI thread to do a time consuming operation. In addition to being terrible design practice, this also has the effect that your DevExpress widget form will hover over top of ALL other applications (since it is a sort of application itself when called from this class), denying your users the ability to multitask with other applications because your application is stealing front and center to show that it is working on something.

You really should use the ThreadPool or a BackgroundWorker or some other type of worker thread to do the complex operation instead. This leaves your main UI thread free to provide status updates on the long operation and avoids this double-UI thread mess above.

J...
  • 30,968
  • 6
  • 66
  • 143
  • Yes, there's basically a hard and fast rule in Windows application development that you **never have more than one UI thread**. Spin off a worker thread to do the work and keep all of the UI on the same thread. And repent from the evil that is `Thread.Sleep`. – Cody Gray - on strike Mar 13 '13 at 11:22
  • @CodyGray - but you CAN have more than one UI thread (mutually independent and exclusive) if you just make another one (second call to `Application.Run()`, as was done here). This just makes a huge mess and really should only be a last-resort kind of solution. The only place I can see it being valid is a meantime-solution to a large body of badly written legacy code which cannot be quickly refactored... sort of a make-do situation until you can get around to fixing the structural problems of the application. – J... Mar 13 '13 at 11:27
  • @CodyGray - rather to say, perhaps, that the rule, albeit not hard and fast, is that you **should** only have one UI thread. If you need to make another one, you're probably doing something wrong! – J... Mar 13 '13 at 11:33
  • That's what I meant to imply with "basically". There's always a bad, messy, broken way to do things. You could accomplish the same thing `Application.Run` is doing for you by just displaying a modal dialog. – Cody Gray - on strike Mar 13 '13 at 12:01
  • 1
    @CodyGray - yes, but in this contex `Application.Run` is being called from a secondary thread, making it a modal dialog with its own independent message loop. I think there's a big difference. It really has an independent UI thread with its own message pump that doesn't depend on the application's main UI thread to deliver messages to it. – J... Mar 13 '13 at 12:13
  • 1
    @soham-dasgupta, @J Glad you got it going. You also need to set the thread to STA or you may run into trouble going forward. – JLScott Mar 14 '13 at 12:56
  • @JLScott - agreed, it should be set to STA. I noted that previously in another answer along this line http://stackoverflow.com/a/11156404/327083 – J... Mar 14 '13 at 13:08
2

You need to call _thread.SetApartmentState(ApartmentState.STA) before starting the thread.

JLScott
  • 96
  • 4