Working with database in the OnExecute event (Indy)

Question

Hi , i have a server with these codes :

procedure TFrmMain.TCPServerExecute(AContext: TIdContext);
begin
      Res := DoRegister(Name,Family,Username,Password);
end;

function TFrmMain.DoRegister(Name,Family,Username,Password:string): bool;
var
  Qry: TSQLQuery;
begin
  Qry := TSQLQuery.Create(nil);
  try
    Qry.SQLConnection := FrmConnect.SQLConnection;
    Qry.SQL.Text :='INSERT INTO `table` ...';
    Qry.ExecSQL();
  finally
    Qry.Free;
  end;
  Result := True;
end;

is there any problem with access to one table in various Threads ? and totally what is dangerous to be used in the Onexecute event ?

Thank you for response friends .

so , is it a true way to make different connections for different threads ?

var
  Qry: TSQLQuery;
  SqlCon: TSQLConnection;
Begin
  SqlCon := TSQLConnection.Create(nil);
  Qry := TSQLQuery.Create(nil);
  try
    SqlCon := FrmConnect.SQLConnection;
    Qry.SQLConnection := SqlCon;
  finally
    SqlCon.Free;
    Qry.Free;
  end;
end;

Answer 1

Yes and no. You can access a single table from different threads, but you need a TSQLConnection instance per thread to do so safely.

Update

Instantiating a different connection for each thread is fine. It's what most webpages do all the time as well (server side scripting using asp, php or ... means state-less execution and thus connections usually do not survive to the next request and have to be re-established).

If you are worried about the overhead, you can consider using a single connection like vcldeveloper suggests. You will have to ensure that any variables and member fields used by that "connection thread" which are changed by the other threads (for example the field member receiving the SQL to be executed), will have to be protected by some sort of synchronisation mechanism.

The same applies to the connection pool as suggested by mjustin, though in that case, the connection pool needs to be protected by synchronisation mechanisms.

Answer 2

Each thread accessing DB should have its own connection, you cannot share a DB connection between several threads. OnExecute event is invoked in the context of the thread corresponding to the requesting client, so each time it is invoked, it is executed inside a worker thread, and such a thread should have its own DB connection.

If you do not want to establish a new connection for each worker thread; one option could be, you dedicate a single thread for DB connection, and delegate all DB operations to that thread, for example your other threads can send their INSERT SQL statements to a queue in that DB thread, and that DB thread executes them one-by-one using a single DB connection. Of course if you take this approach, all DB load would be on a single thread, and if you have so many DB operations, then that DB thread itself could be a performance bottleneck! What's more, taking this approach, query executions would be asynchronous except you use a synchronization technique whenever each of your threads ask the DB thread to execute a DB query for them.

Also take note that if your DB access components are ADO, then you have to call CoInitialize and CoUninitialize, because Delphi runtime only does that for the main thread not other threads which are created by you.

Answer 3

I would use a connection pool for the database connections. Every thread then only requests a connection from the pool when needed (which might block if there are currently no free connections in the pool) and then uses and finally returns it to the pool. A pool has the advantage that there are less connections needed than there are concurrent threads, and the connections are already present when needed.

Answer 4

Your second code fragment is not correct. You're overwriting the new connection with the global connect, when you should be copying out the connection string. You're also freeing that global which will probably cause problems for the rest of your application. Something like this, depending on the details of your TSQLConnection class:

SqlCon := TSQLConnection.Create(nil); // create
Qry := TSQLQuery.Create(nil);
try
  //SqlCon := FrmConnect.SQLConnection; // overwrite!!!
  SqlCon.ConnectionString :=  FrmConnect.SQLConnection.ConnectionString;
  SqlCon.Active := true;
  Qry.SQLConnection := SqlCon;
  ...

If you want to have a databse connection pool it's quite tricky because the connections are usually thread-specific - you need one per thread and you can't pass them between threads. So you end up writing a lot of code to support that.

I now use the OmniThreadLibrary and have a factory method that returns a new database connection. That gives me a thread pool that I feed tasks into, so my specific task is bound to an existing thread when it executes, but the thread is fairly long-lived. The code I had to write to get this is very small (I'm using ADO):

type
    // a factory to generate new instances of our thread-specific data
    IThreadPoolData = interface
        ['{14917B01-6613-4737-B87E-0046789D4284}']
        function GetConnection: TADOConnection;
        function GetStoredProc: TADOStoredProc;
    end;

    TThreadPoolData = class(TInterfacedObject, IThreadPoolData)
    strict private
        FADOConnection: TADOConnection;
        FStoredProc: TADOStoredProc; // lazy creation!
    public
        constructor Create(aConnectionString: string); overload;
        destructor Destroy; override;
        function GetConnection: TADOConnection;
        function GetStoredProc: TADOStoredProc;
    end;

// create the connection here so thread creation is slow but using it 
// is (relatively) fast

constructor TThreadPoolData.Create(aConnectionString: string);
begin
    FADOConnection := TADOConnection.Create(nil);
    FADOConnection.LoginPrompt := false;
    FADOConnection.ConnectionString := aConnectionString;
    FADOConnection.ConnectOptions := coAsyncConnect;
    FADOConnection.Connected := true;
end;

destructor TThreadPoolData.Destroy;
begin
    FADOConnection.Connected := false;
    if assigned(FStoredProc) then
        FreeAndNil(FStoredProc);
    FreeAndNil(FADOConnection);
end;

You will need to do something similar if you write your own thread or connection pool.