0

I have created a singleton class for holding the Firebird database connection and I'm executing queries (see below), which worked like a charm, until I had to add to one of the applications some timers to set up & check some data regularly.

Now very often I get error saying that "Parallel transactions are not supported". I can't get it, because I got closing of the connection in finally block.

I already spent few very long evenings on searching the web and trying to figure out how to approach, but it seems I need support.

Here is the main part of my fBird class:

public sealed class fBird {
    private FbConnection FbConn = new FbConnection(connectionString);
    static fBird m_oInstance = null;
    static readonly object m_oPadLock = new object();

    public static fBird Instance {
        get {
            lock (m_oPadLock) {
                if (m_oInstance == null) {
                    m_oInstance = new fBird();
                }
                return m_oInstance;
            }
        }
    }

    //Data query:
    public DataTable qSelect(string query){
        if(!getState().Equals("Open")) FbConn.Open();
        using (FbTransaction transaction = FbConn.BeginTransaction()) {
            try{
                FbCommand cmd = new FbCommand(query, FbConn, transaction);
                FbDataAdapter adpt = new FbDataAdapter(cmd);
                DataTable dt = new DataTable();
                adpt.Fill(dt);
                transaction.Commit();
                return dt;
            } catch (Exception ex){
                transaction.Rollback();
                throw new Exception("DataQuery: " + ex.Message);
            } finally {
                FbConn.Close();
            }
        }
    }

    //NonQuery query:
    public int NonQuery(string query){
        if(!getState().Equals("Open")) FbConn.Open();
        using (FbTransaction transaction = FbConn.BeginTransaction()) {
            try{
                FbCommand cmd = new FbCommand(query, FbConn, transaction);
                int i = cmd.ExecuteNonQuery();
                transaction.Commit();
                return i;
            } catch (Exception ex){
                transaction.Rollback();
                throw new Exception("NonQuery: "  + ex.Message);
            } finally {
                FbConn.Close();
            }
        }
    }
}

Here is how I use the class:

DataTable dt = fBird.Instance.qSelect("SELECT * FROM USERS");
int i = fBird.Instance.NonQuery("DELETE FROM TABLE");

Here is how I use timers:

public partial class MainForm : Form {
    private System.Timers.Timer aTimer;

    ...

    void setTimers() {
        aTimer = new System.Timers.Timer(1500));
        aTimer.Elapsed += OnTimedEvent;
        aTimer.AutoReset = true;
        aTimer.Enabled = true;
    }

    void OnTimedEvent(Object source, ElapsedEventArgs e) {
        try{

            //different changes to & reads from database

        } catch(Exception ex) {
            MessageBox.Show("OnTimedEvent: " + ex.Message);
        }
    }
}

What I have found out is that for some reason there is more than one connection to Firebird made (singleton shouldn't allow that, right?). I can see it in Windows Performance Monitor on Network tab. As a testing I do very different operations, open different windows & screens in application, also save some changes. At random I got error (parallel transactions) and new connection is created and than for some time (like 30-40 clicks) all works fine.

It just drives me crazy. I hope someone of you will be able to help me. I have feeling that I do something really wrong here, but I just can't find it. Let me know if you need any more information.

Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
  • Possible duplicate of [getting db connection through singleton class](https://stackoverflow.com/questions/814206/getting-db-connection-through-singleton-class) – John Wu Mar 26 '18 at 17:46
  • Why are you using a singleton with the database connection? Is it a single user license? Seems to me you want to allow multiple connections. – John Wu Mar 26 '18 at 17:47
  • I have raised in school that was saying multiple connections to database are evil and should be avoided, but now, as i read more it seems that it is not the truth... can you propose material on net i could read to get myself more correct in this manner? – Przemek Kwiecień Mar 26 '18 at 18:12
  • Forget singleton, banish `static`. When each thread has its own connection you're safe. – H H Mar 26 '18 at 18:49
  • Thank you all for commiting. I have kicked off some of the code, removed singleton, made it static and started to use connection pooling. Since 2 days I had no issues with database connections and some of windows are loading faster i think. – Przemek Kwiecień Mar 29 '18 at 17:29

3 Answers3

2

If you are using multiple threads, then you should not share a single connection. Your problem is not one of multiple connections, but of multiple threads using the same connection, and trying to create their own transactions, which is explicitly not supported by the driver. Your code is likely to suffer from other race conditions as well.

Stop using that singleton. Instead obtain a new connection for each unit of work and close it when you're done with it. The Firebird ADO.net provider by defaults provides connection pooling, so it is relatively cheap to do.

Personally, I would get rid entirely of that fBird class, but otherwise at minimum get rid of the shared instance, implement IDisposable to close the connection on dispose, add a static method to create instances on demand. You should then use those fBird instances in a using to make sure it gets correctly closed at the end.

The reason why I suggest to get rid of that fBird class, is that you are just wrapping the normal connection object, and providing a poorer and more brittle abstraction.

Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
1

You need to use a lock, singleton does not guarentree a single user

Christian Sauer
  • 10,351
  • 10
  • 53
  • 85
0

There are three things about your fBird class that you may want to rethink.

Firstly, as other answers have pointed out, the FBConnection object is not threadsafe. (That's a Firebird design/restriction, not a C# library specific behaviour). That means that you may either:

  • Ensure that each of your threads gets their own instance of this class; OR
  • Ensure that access to your single connection is serialised (ie use a lock())

Secondly, your methods are very heavy as you are doing a full connect/disconnect each time. That will really slow things down when you turn up the volume.

Thirdly, you are always committing in NonQuery. If your operation involves more than one query then your database may be left inconsistent if there is a failure between the two.

Here is an idea as a compromise solution for my first two points. In one of my old applications (not c# sorry), I created a connection broker to reduce the connect/disconnect overhead in a restful service. This had a maximum concurrent connections configuration value.

You write a GetConnection and ReturnConnection method.

The GetConnection internally locks while it determines whether an existing connection is available. It then flags it as in use and returns it. Where no connection is available, it then establishes a new one (lazy create), flags it as in use and returns it (providing you are under the maximum limit).

The ReturnConnection internally locks then flags it as available. There is an optional parameter on return to state if the connection is suspect (eg you had a socket error when using it). The other thing ReturnConnection does is to timestamp it. A background process periodically closes connections above a certain count if they haven't been used for a while.

GetConnection guarantees the transaction is not started. ReturnConnection throws an exception if it is in transaction (caller must commit, rollback, or indicate there was a problem) or if the connection has been closed.

Adam G
  • 1,283
  • 1
  • 6
  • 15