0

I know that Thread.Abort() is not really safe, but I cannot imagine what it could cause in case below:

Private threadLoadList As Thread

Private Sub LoadList(ByVal argument As Integer)

    Try
        Refresh_Grid(GET_DATA(argument), argument) 'Fills grid with data loaded from database
        Change_Cursor() 'Changes cursor to Cursor.Default
    Catch ex As ThreadAbortException
        'Do nothing
    Catch ex As Exception
        Error_log(ex.Message) ' Saves ex.Message in database
    End Try

End Sub

Private Sub SomeValueChanged(sender As Object, e As EventArgs) Handles Control.ValueChanged
    If Not threadLoadList Is Nothing Then
         If threadLoadList.IsAlive Then
              threadLoadList.Abort()
         End If
     End If
     Cursor = Cursors.WaitCursor
     threadLoadList = New Thread(AddressOf LoadList)
     threadLoadList.Start(1)
 End Sub

As you can see, user can change some value (ComboBox) and in result change content of the grid. Function GET_DATA() takes around 10 seconds, thus it is possible that user changes value of Combobox before grid is refreshed - that is why previously started thread is killed.

Is it dangerous? If yes, could you propose some other solution?


Ok, I understand ;). I try to avoid timeouts (in some cases query executes below 1 second) Is it better solution:

Private threadLoadList As Thread
Private Changed As Boolean = False

Private Sub LoadList(ByVal argument As Integer)

    Try
        Dim dt As DataTable = GET_DATA(argument)
        'Enter Monitor
        If Changed Then
            Return
        End IF
        'Exit Monitor
        Refresh_Grid(dt, argument) 'Fills grid with data loaded from database
        Change_Cursor() 'Changes cursor to Cursor.Default
        'Enter Monitor
            Changed = False
        'Exit Monitor
    Catch ex As ThreadAbortException
        'Do nothing
    Catch ex As Exception
        Error_log(ex.Message) ' Saves ex.Message in database
End Try

End Sub

Private Sub SomeValueChanged(sender As Object, e As EventArgs) Handles Control.ValueChanged
     'Enter Monitor
        Changed = True
     'Exit Monitor
     Cursor = Cursors.WaitCursor
     Dim t As Thread = New Thread(AddressOf LoadList)
     t.Start(1)
 End Sub
Andrew Barber
  • 39,603
  • 20
  • 94
  • 123
Maciej Nowicki
  • 237
  • 1
  • 13
  • 1
    Yes it is dangerous. What if the AbortException happens inside a finally clause inside Refresh_Grid() ? – H H Jul 23 '13 at 08:53

1 Answers1

1

First things first, unless there is extra code in Refresh_Grid that I am not seeing it looks like you are attempting to modify a UI element from a thread other than the UI thread. That is a big no-no. UI elements are designed to be accessed only from the UI thread so attempting to modify one on a worker thread will lead to unpredictable behavior.

In regards to your usage of Thread.Abort the answer is yes; it is dangerous. Well, it is dangerous in the sense that your application will throw an exception, corrupt data, crash, tear a hole in spacetime, etc. There are all kinds of failure modes that might be expected from aborting a thread. The best solution is to use a cooperative termination protocol instead of doing a hard abort. Refer to my answer here for a brief introduction on valid approaches.

Community
  • 1
  • 1
Brian Gideon
  • 47,849
  • 13
  • 107
  • 150
  • Thank you Brian, Don't worry about UI thread - I'm familiar with the problem and I know how to use InvokeRequired etc. Finally I used some kind of 'Poll a stopping flag' mechanism similar to the one presented in my edited post: After database query I check if the result is still expected - if not I finish the thread immediately via Return. Otherwise thread continue execution inside the monitor, thus applicaion is free of Race Condition. – Maciej Nowicki Jul 24 '13 at 08:25