[RESOLVED] Threading best practices...help!

[BigPhil]

Hello everyone! I have a couple of questions about some threading best practices. Here is some of the code I have written that spawns some threadpool threads and each threadpool thread opens a log file on processes it (still dont know if its any faster than a single thread at this point) >

Imports System.IO
Imports System.Data.SqlClient
Imports System.Threading

Public Class Form1
    Dim SqlDataConn As New SqlClient.SqlConnection(My.Settings.SqlDataConn)
    Dim CustomSql As New SqlDataRetrieve.SqlReadOrChange
    Dim LogsScanned, MaxThreads, PlaceHolder, AvailableThreads As Integer
    Dim TempLogsDir As String = "TempLogs"
    Dim SqlLock As New Object

    Private Sub ButtonReadLogs_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles ButtonReadLogs.Click
        WriteToLogFile(Now.ToString() + " -Process started")
        SqlDataConn.Open()
        My.Computer.FileSystem.CreateDirectory(TempLogsDir)
        For Each LogFile As String In My.Computer.FileSystem.GetFiles(Me.TextBoxLogPath.Text, FileIO.SearchOption.SearchTopLevelOnly)
            Dim LogFileInfo As New FileInfo(LogFile)
            If Not My.Computer.FileSystem.FileExists(TempLogsDir + "\" + LogFileInfo.Name) Then
                My.Computer.FileSystem.CopyFile(LogFile, TempLogsDir + "\" + LogFileInfo.Name)
            End If
            LogFile = Nothing
            LogFileInfo = Nothing
        Next LogFile

        For Each CopiedLogFile In My.Computer.FileSystem.GetFiles(TempLogsDir)
            ThreadPool.QueueUserWorkItem(New WaitCallback(AddressOf ReadLogFile), CopiedLogFile)
            LogsScanned += 1
            CopiedLogFile = Nothing
        Next CopiedLogFile

        ThreadPool.GetMaxThreads(MaxThreads, PlaceHolder)
        ThreadPool.GetAvailableThreads(AvailableThreads, PlaceHolder)
        Do While MaxThreads <> AvailableThreads
            ThreadPool.GetMaxThreads(MaxThreads, PlaceHolder)
            ThreadPool.GetAvailableThreads(AvailableThreads, PlaceHolder)
            Thread.Sleep(5000)
        Loop
        WriteToLogFile(Now.ToString() + " -Process Finished")
        MessageBox.Show("Finished!")
    End Sub

    Private Sub ReadLogFile(ByVal state As Object)
        Dim LogFilePath As String = DirectCast(state, String)
        Dim MyCommand As New SqlCommand
        Dim oRead As System.IO.StreamReader
        Try
            oRead = File.OpenText(LogFilePath)
            While oRead.Peek <> -1
                Try
                    Dim LogFileLine As String = oRead.ReadLine.Replace("'", "`")
                    If Not LogFileLine.StartsWith("#") Then
                        If Not LogFileLine.Contains("EVJournaling") Then
                            LogFileLine = LogFileLine.Replace(",", "','")
                            LogFileLine = LogFileLine.Insert(0, "'")
                            LogFileLine = LogFileLine + "'"
                            MyCommand.CommandText = ("sp_InsertMailEvent " + LogFileLine)
                            SyncLock SqlLock
                                CustomSql.InsertUpdateDelete(SqlDataConn, MyCommand)
                            End SyncLock
                            LogFileLine = String.Empty
                            MyCommand.CommandText = String.Empty
                        Else
                            LogFileLine = String.Empty
                        End If

                    End If
                Catch ex As Exception

                End Try
            End While
            oRead.Close()
            oRead.Dispose()
        Catch ex As Exception
         
        End Try
        MyCommand.Dispose()
        LogFilePath = Nothing
        state = Nothing
    End Sub

    Private Sub WriteToLogFile(ByVal Text As String)
       File.AppendAllText("ExchangeServerMT_" & DateString & ".log", Text & ControlChars.CrLf)
        Text = Nothing
       End Sub

End Class

Here is the code in the CustomSql library >

Imports System.Data.SqlClient

Public NotInheritable Class SqlReadOrChange

    ''' <summary>
    ''' Connects to a SQL database, executes a SqlCommand, and returns the number of rows affected.
    ''' </summary>
    ''' <param name="MyConnection">The connection used to open the SQL Server database</param>
    ''' <param name="MyCommand">The SqlCommand to run against the connection</param>
    ''' <param name="KeepConnectionOpen">Determines whether or not to keep the connection to the database open. Default is True.</param>
    ''' <remarks></remarks>
    Public Function InsertUpdateDelete(ByRef MyConnection As SqlConnection, ByVal MyCommand As SqlCommand, Optional ByVal KeepConnectionOpen As Boolean = True) As Integer
        Try
            MyCommand.Connection = MyConnection
            If MyConnection.State <> ConnectionState.Open Then
                MyConnection.Open()
            End If
            InsertUpdateDelete = MyCommand.ExecuteNonQuery()
            If KeepConnectionOpen = False Then
                If MyConnection.State <> ConnectionState.Closed Then
                    MyConnection.Close()
                End If
            End If
        Catch ex As Exception
            InsertUpdateDelete = Nothing
        End Try
        MyCommand.Dispose()
    End Function
End Class

My question is this; Is it necessary to use some sort of synchronizing (ie: im using SyncLock in the ReadLogFile sub) when making the insert call into the CustomSql class? If I dont use it, I get some strange NullException errors in the CustomSql class. I'm still a bit confused on WHEN I should use some sort of synchronizing when working with multi threading. Like when each threadpool thread gets called, does each one keep track of all its own variables in the ReadLogFile sub, so there is no contamination of values between the threads? I understand the simple use of synchronizing like this article multithreading, but would like some more help understanding using it in more complicated code. Thanks in advance for any help you can give me!

[chris128]

Like when each threadpool thread gets called, does each one keep track of all its own variables in the ReadLogFile sub, so there is no contamination of values between the threads?

yeah exactly, each thread has its own variables if they are declared within that method that the thread is executing - however if you are accessing objects that are declared at class level from multiple threads at the same time then you can run into problems if one thread modifies the object whilst another thread is trying to read it. A common scenario is with a list/array declared at class level, as one thread might be looping through it whilst another thread removes an item and then you will get an exception from the initial thread as the collection has been modified during the loop.

I think when working with SQL connections from multiple threads you need to have an SqlConnection object for each thread rather than having them all sharing one (unless you use SyncLock to ensure only one thread is using it at once), but you are already doing that in your code so I'm not sure why you would get any problems. Where exactly do you get the Null Reference Exceptions thrown and which object are they referring to?

[BigPhil]

Chris,

Thanks for your input on this, and confirming my assumption in your first paragraph. So when I use the SyncLock, everything runs fine, but if I remove it, I get the NullReferenceException in the SqlReadOrChange class at the line "InsertUpdateDelete = MyCommand.ExecuteNonQuery()". I wonder...do you think it would be better to reference the "MyConnection" varialbe in the SqlReadOrChange class ByVal instead of ByRef so each thread has its own connection?...and then just have each one open and close the connection after each insert? Im also wondering if I am gaining any performance by using a ThreadPool in this code or if it would be better to just use a single thread for the background operations? what do you think? thanks again for the help!

[chris128]

do you think it would be better to reference the "MyConnection" varialbe in the SqlReadOrChange class ByVal instead of ByRef so each thread has its own connection?

This is covered in detail in quite a few articles on the web (and posts on this forum) but I'll give you a fairly simple version: passing an instance of a class ByRef does very little different to passing it ByVal. If it was an instance of a structure (Value Type) then it would be different, but with a class (Reference Type) the main difference is that you can set the original instance to Nothing/Null but that is about it - modifying the instance from within the method where you passed it in ByVal/ByRef will still affect the original instance no matter which way you passed it in. So ByVal/ByRef does not affect whether or not each thread running that method would have its own copy of the object, they are all working with the same copy if you pass an instance of a class in to a method that multiple threads are going to run. This doesnt have anything to do with threading - this is the same for any arguments that you pass in to any method. Take this very basic example:

Private Sub Button1_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button1.Click
        Dim SqlConn As New SqlClient.SqlConnection
        SqlConn.ConnectionString = "Database=C:\ORIGINALdatabase.mdb"
        SomeMethod(SqlConn)
        MessageBox.Show(SqlConn.ConnectionString)
        'Messagebox shows Database=C:\NEWdatabase.mdb
End Sub
 
Private Sub SomeMethod(ByVal sql As SqlClient.SqlConnection)
        sql.ConnectionString = "Database=C:\NEWdatabase.mdb"
End Sub

As I've mentioned in the comments, when that MessageBox is shown you can see that the connection string of the original SqlConnection instance has been modified even though the object was passed in ByVal to the method... and its exactly the same when working with multiple threads. The exception to this rule, as I've mentioned, is Structures as they are Value Types so if you pass them ByVal then you actually are passing a copy of the original object in.

Obviously this does not apply to any objects that are created within the method/thread - they are all totally separate.

So with all that in mind, take a look through your code and try and identify points where that might cause problems. Though from taking a quick look at your code it doesnt look like that is what is causing the problems with your Null Reference error, as when you call your InsertUpdateDelete function you are already inside the method that is run by separate threads and dont appear to be using any objects that are shared between them, other than the SqlConnection object so it might be worth just trying creating the SqlConnection within your method that is run on multiple threads - see one of my old threads for a little bit of info on that: http://www.vbforums.com/showthread.php?t=589543 (post #2 specifically)

Oh and also I would try to get away from returning values for Functions in the way you are currently doing it. Its much more standard practice to just use Return rather than setting the function name to a value as you are doing. Note sure what (if any) difference there is technically but 90% of people tend to use Return so its worth getting into the habbit of doing it the same as everyone else to avoid confusion.

Hope that helps

EDIT: For more info on the ByVal/ByRef issue you might want to have a look at this article NickThissen wrote recently, though I have to admit I have not read it all yet... but he's a smart chap so I assume it is all technically correct http://www.vbforums.com/showthread.php?t=612831
The most important paragraph perhaps being this one:

So while VB actually does make a copy of the value, that value is only the memory location for a reference type, and the actual data on that memory location is not copied elsewhere. So, when you make a change to a reference type, even when passed ByVal, you are changing the data that is stored on the memory location given by the copy of the value of the reference type. Since the copy of the value of the reference type is just the same memory location, you are actually accessing the same data!
This is the crucial point. You are accessing the same data, regardless of whether you pass the argument ByVal or ByRef. So, whenever you pass a reference type by value (or by reference of course), you can change it!

[BigPhil]

Thanks for the detailed information, Chris...I appreciate it! I see what your are saying about byval/byref and I'll read the article you posted. I had no idea that it was working that way! I just read your post about the similar issue you were running into with threadpool threads and sql connections...thanks for the link as it explains it perfectly! as far as returning the functions the way I do, I have used both styles. I usually set the function to the result because it seems that when you use Return, this stops any further execution in the function. sometimes I wish to process further, for cleanup and disposing of objects and so on. I will look into it. Thanks again for the help!