Question Intermittent "ConnectionString property has not been initialized." error

[tracertong]

Hello,

I've recently inherited a VB.NET forms project which does basic background administrative tasks on a Windows Server 2003 machine (virtual machine, FWIW). Part of it's functionality is a Timer control, whose tick is set at every 10 seconds, and runs all the time. This tick event writes or updates a progress record into a database running locally on the same machine, using a Non-query call.

Normally, this runs fine, but very occasionally (say between one to five occasions, every hour) it throws a "ConnectionString property has not been initialized." error out.

This is the method it is calling, to write the progress query to the database (I haven't shown the call, because it is this that appears to be the problem):

    Public Function doNonQuery(ByVal strQuery As String)
        Dim result As Integer
        myConnection = New OdbcConnection()
        myConnection.ConnectionString = "dsn=sffdsn"
        myCommand = New OdbcCommand(strQuery, myConnection)
        myConnection.ConnectionTimeout = 0
        myCommand.CommandTimeout = 0
        Do Until myConnection.State = ConnectionState.Open
            myConnection.Open()                 <-- The error throws here
        Loop
        result = myCommand.ExecuteNonQuery()
        myConnection.Close()
        myConnection.Dispose()
        Return result
    End Function

Now, this may be a little sloppy, I don't know (I'm not strong on .NET, as yet), but I don't see how the ConnectionString property can not be initialized, when it appears to be hard-coded ("sffdsn" is a system DSN, by the way). I also don't see why this might happen as infrequently as 1 in 600 occasions that it is called.

My predecessor appears to have 'handled' this problem, by wrapping the call in a Try, with an empty catch - but since any number of other things could escape attention, this way, I decided to catch it properly, and log the exception message. The problem is, the exception message is a fairly typical ".NET barf", of around 30 lines, and (intermittent as it is) logging the error is making my log almost twice as big as it was before. In the 11 hours since midnight, for instance, I have 42 examples of this happening, and the 42 records dwarf the rest of the log. (The other problem is, that I don't know enough .NET to understand why the exception is happening, of course!)

 

[cjard]

Mmmm.. might have been handy to actually see the errors you logged, but..

I'd never repeatedly try to open a DB connection in a loop.. For this particular bit of code the key may lie elsewhere that we cannot see. We'll probably evetually discover after much finger-wear in posting questions about it, that this is declared in a shared module, myConnection is a module wide variable accessible to all threads currently executing, one thread is stuck in a loop hammering the Open() function several thousand times a second, for more than 10 seconds when another thread spawned off the timer (becuase the first one is still stuck in a loop) sneaks in, renews the myConnection to something brand new and thread 2 doesnt quite get to the part where it actually sets the connection string before thread 1 (that has just had the rug pulled from under its feet) tries another Open() as it hammers away, and lo and behold: we're trying to open in one thread a connection that another thread has just ripped the guts out of...

If I were doing this, I would use the dataset designer to make myself a TableAdapter embodying the connection and the command. I'd then have a timer that did this:

Public Sub Click(blah) Handles Timer1.Click
  If m_dbIsBusy Then Return

  Try
    m_dbIsBusy = True
    Dim ta a New TableAdapter()
    ta.DoThe10SecondUpdateThing()
  Catch ex as Exception
    Log(ex.Message & " from trace: " & ex.StackTrace)
  Finally
    m_dbIsBusy = false
  End Try
End Sub

No way I would ever put an event handler into a loop where it can get stuck, wouldnt have it renew any shared or class wide variables it would use, and I wouldnt tie it up for an inordinately long time doing things.. Be really careful that your query will never take longer than 10 seconds as you'll then have your timer starting another query that will take longer than the first (because the db is doing both), then another.. and another. Gradually it gets so long that the requests just keep piling in every 10 seconds and you kill your db

In the code i posted above, a flag is set before doing any work and only if the flag is unset, should the db query run.. You could enhance this to kicking off a backgroundworker only if it's not busy:

 

[tracertong]

The limited notes seem to indicate that he runs the connect in a loop, in order to stop some other error happening.

Sure enough, I REM the loop out, and I start getting errors from Close being called on a connection in as state of opening, or ExecuteReader being called on a connection that is still connecting, and so on... Again, this isn't a show stopper: the module actually does it's work, and I've only had a dozen such errors in the course of the forty or fifty minutes I've been playing around with it in this state.

However, yes, it looks like myConnection is a shared class-level property, so what you say about threads hijacking it, will almost certainly be what is amiss. I'm guessing he's tried to debug it in step-through mode, or something, and, unable to produce the race condition in those circumstance, he's finally given up - hence the unhandled try and the loop.

Looks like a rewrite is called for, but since it's only cluttering the log at the moment, it can wait until after the weekend.

Thanks

 

[jmcilhinney]

You are creating, opening, using, closing and destroying a connection all within the one method, so why are you not using a local variable? It would be very bad design for that variable to be accessed outside that method so it should exist only inside the method:

Using connection As New OdbcConnection("connection string here"),
      command As New OdbcCommand("SQL code here", connection)
    connection.Open()

    Return command.ExecuteNonQuery()
End Using

The variable exists only within the scope of the Using block and the object is guaranteed to be disposed.

I don't really understand the loop anyway. I didn't think it was possible for Open to complete successfully without the connection being opened. If you call Open and afterwards the ConnectionState is not Open, I'm fairly sure that an exception would have to have been thrown.

 

[tracertong]

Thanks. I rewrote the entire class, in the end: it's a shell of it's former self, but a much nicer shell. Writing everything in scope, like this, is very Perl-like, so I'm happy with that. I suspect I'll have to take the 'refactoring-scissors' to a fair bit of this code!

I suppose what's surprising - and what threw me, initially (not knowing good syntax from bad) was that anything so wrong, should go wrong so infrequently... but this is the curse of fault-tolerant technologies.