Jacek Laskowski 57 Posted October 6, 2020 Hi, I found a serious FireDAC bug, I wrote a minimal sample program to reveal it, you need a Firebird server and any database to run. Maybe there is also a problem with other servers, I did not check. The problem occurs when you do not explicitly assign the FDConnection control to the FDQuery, but use the registered Connection Definition Name and assign it by Query.ConnectionName and occurs when Connection Definition Name is used more than once at a time (in thread). The result is a mass of random errors, each time you run it, there are different errors, in a different place of FireDAC code. Source of example, console app: program FDPoolingBug; uses System.Threading, System.Classes, Winapi.Windows, FireDAC.Phys.IBWrapper, FireDAC.Phys, FireDAC.Phys.FBDef, FireDAC.Stan.Intf, FireDAC.Stan.Def, FireDAC.DApt, FireDAC.Phys.FB, FireDAC.Stan.Pool, FireDAC.Stan.Async, FireDAC.Comp.Client, System.SysUtils; {$APPTYPE CONSOLE} {$R *.res} const CONN_DEF_NAME = 'CONNECTION_TO_DB'; var bEnd : Boolean = False; procedure RegisterConnectionDefinition(); var oConDef: IFDStanConnectionDef; oPars: TFDPhysFBConnectionDefParams; begin if FDManager.ConnectionDefs.FindConnectionDef(CONN_DEF_NAME) = nil then begin oConDef := FDManager.ConnectionDefs.AddConnectionDef; oConDef.Name := CONN_DEF_NAME; oPars := TFDPhysFBConnectionDefParams(oConDef.Params); oPars.DriverID := 'FB'; oPars.Server := '192.168.1.10'; oPars.Database := 'Some_FB_Database.FDB'; oPars.Port := 3060; oPars.Pooled := True; oPars.PoolMaximumItems := 100; oPars.PoolCleanupTimeout := 30000; oPars.PoolExpireTimeout := 90000; oPars.UserName := 'sysdba'; oPars.Password := 'masterkey'; oPars.SQLDialect := 3; oPars.Protocol := ipTCPIP; oPars.CharacterSet := TIBCharacterSet.csUTF8; oConDef.Apply; end end; procedure QueryWithAutoConnection(); //This method works incorrectly var lCounter: Integer; lId: Integer; lQuery: TFDQuery; begin lId := GetCurrentThreadId() mod 99; Writeln('start: ', lId); try lQuery := TFDQuery.Create(nil); lQuery.ConnectionName := CONN_DEF_NAME; lQuery.SQL.Text := 'select * from rdb$database'; lCounter := 0; repeat lQuery.Open; lQuery.Close; Inc(lCounter); if lCounter = 1000 then begin lCounter := 0; Writeln(lId); end; until bEnd; except On E : Exception do begin writeln(Format('id: %d, Exception: %s, Desc: %s', [lId, E.ClassName, E.Message])); Writeln; end; end; end; procedure QueryWithDedicatedConnection(); // This method works correctly var lConn: TFDConnection; lCounter: Integer; lId: Integer; lQuery: TFDQuery; begin lId := GetCurrentThreadId() mod 99; Writeln('start: ', lId); try lConn := TFDConnection.Create(nil); lConn.ConnectionDefName := CONN_DEF_NAME; lQuery := TFDQuery.Create(nil); lQuery.Connection := lConn; lQuery.SQL.Text := 'select * from rdb$database'; lCounter := 0; repeat lQuery.Open; lQuery.Close; Inc(lCounter); if lCounter = 1000 then begin lCounter := 0; Writeln(lId); end; until bEnd; except On E : Exception do begin writeln(Format('id: %d, Exception: %s, Desc: %s', [lId, E.ClassName, E.Message])); Writeln; end; end; end; procedure RunOne(); var aTask: ITask; begin aTask := TTask.Create(QueryWithAutoConnection); aTask.Start; TTask.WaitForAll([aTask]); end; procedure RunParallel(lThreadCount : Integer); var aTasks: TArray<ITask>; i: Integer; begin SetLength(aTasks, lThreadCount); for i := 0 to Pred(lThreadCount) do begin aTasks[i] := TTask.Create(QueryWithAutoConnection); Sleep(10); aTasks[i].Start; end; TTask.WaitForAll(aTasks); end; begin try RegisterConnectionDefinition(); //RunOne; RunParallel(3); Readln; except on E: Exception do Writeln(E.ClassName, ': ', E.Message); end; end. Can someone check it in Delphi 10.4.1? I use Delphi 10.3. Share this post Link to post
Lajos Juhász 293 Posted October 6, 2020 This is the documented behavior: http://docs.embarcadero.com/products/rad_studio/firedac/frames.html?frmname=topic&frmfile=Multi_Threading.html. FireDAC is thread-safe, when the following conditions are meat: A connection object and all associated with it objects (like TADQuery, TADTransaction, etc) in each moment of time must be used by a single thread. ADManager must be activated before threads will start by setting ADManager.Active to True. Means, after a thread opened a query, and until its processing is not finished, application cannot use this query and connection objects in an other thread. Or, after a thread started a transaction, and until it is not finished, application cannot use this transaction and connection objects in an other thread. Practically this means an application must serialize access to a connection across all threads, and that is not a convenient technique. Breaking these rules may lead to misbehavior, AV's and errors like the SQL Server error "Connection is busy with results for another command". Share this post Link to post
Jacek Laskowski 57 Posted October 6, 2020 (edited) Ok, I forgot FDManager.Active := True; at the end of the RegisterConnectionDefinition() method, but it doesn't change anything, after adding the errors are the same. And as for the connection... I don't use the same connection, I use a pool that is managed by FireDAC, so I assume that for every query it gives a new connection, isn't that how it should work? from documentatio9n: "The ConnectionName property value must be specified before Prepare call. If it matches the name of one of connection definitions, then FireDAC will transparently create connection object and link the dataset with it. " Edited October 6, 2020 by Jacek Laskowski Share this post Link to post
Josep 8 Posted October 7, 2020 Is threade safe declare "bEnd" an used as global var? var bEnd : Boolean = False; Share this post Link to post
Lajos Juhász 293 Posted October 7, 2020 20 hours ago, Jacek Laskowski said: from documentatio9n: "The ConnectionName property value must be specified before Prepare call. If it matches the name of one of connection definitions, then FireDAC will transparently create connection object and link the dataset with it. " I don't know where is this written in the documentation. However, of course this is true only for the first time. Like in BDE,as FD was designed to replace BDE, later when you give the same connectionname it will use the already created connection. There is no reason to create a connection for every single query as it would be inneficient with databases. You can check it simple using code: FDQuery1.ConnectionName:=CONN_DEF_NAME; FDQuery1.Open; FDQuery1.Connection.Name:='Test'; // Just to give a name. FDQuery2.ConnectionName:=CONN_DEF_NAME; FDQuery2.Open; if FDQuery1.connection=fdQuery2.Connection then ShowMessage('FDQuery1 shares the connection with FDQuery2.'); Share this post Link to post
Dmitry Arefiev 101 Posted October 7, 2020 When FDQuery's use ConnectionName, then single temporary TFDConnection will be created transparently and used by all FDQuery's, etc Share this post Link to post
Jacek Laskowski 57 Posted October 7, 2020 18 minutes ago, Dmitry Arefiev said: When FDQuery's use ConnectionName, then single temporary TFDConnection will be created transparently and used by all FDQuery's, etc And unfortunately this is a certain inconsistency, because when I create FDConnection and assign it ConnectionDefName, a physical, new connection is taken from the pool. Why doesn't he do it with Query if he gets a new, automatically created FDConnection in the same way? Couldn't this new FDConnection also get a new, unique connection from the pool? Share this post Link to post
Dmitry Arefiev 101 Posted October 7, 2020 You mean, that each FDQuery will get own connection from pool ? If yes, then this will lead to a lot of issues ... Share this post Link to post
Uwe Raabe 2057 Posted October 7, 2020 I am with Dmitry here. Having a separate connection per query is definitely wrong. Interestingly I never made use of that TFDQuery.ConnectionName feature. Instead I have a single TFDConnection instance for a specific part of the program where all necessary queries are connected to. Thus all these queries use the same connection and it works perfectly even in multi-threaded situations. I admit that identifying those groups of queries sharing one connection may sometimes be a bit tricky. Share this post Link to post
mvanrijnen 123 Posted October 7, 2020 (edited) 52 minutes ago, Uwe Raabe said: even in multi-threaded situations. I That is tricky, you should not do that, maybe the underlaying physicaldb class takes care of you for this, but with most db types, you must not practise this. (i can not forbid you todo 🙂 ). See this: http://docwiki.embarcadero.com/RADStudio/Rio/en/Multithreading_(FireDAC) I have some troubles with a new project too, with MT and FD, probably some (yet) unidentified loading from a thread in a datamodule which is purely running mainthread. Quote FireDAC is thread-safe if the following conditions are met: A connection object and all objects associated with it (such as TFDQuery, TFDTransaction, and so on) are used by a single thread at each moment. FDManager is activated before threads start, by setting FDManager.Active to True. Edited October 7, 2020 by mvanrijnen Share this post Link to post
Uwe Raabe 2057 Posted October 7, 2020 6 minutes ago, mvanrijnen said: See this: http://docwiki.embarcadero.com/RADStudio/Rio/en/Multithreading_(FireDAC) That is exactly how I do it. The TFDConnection has the proper ConnectionDefName set. Then, inside the same thread, I open the connection, do all the query stuff and close the connection. All the queries linked to that connection are only used inside that same thread. Share this post Link to post
mvanrijnen 123 Posted October 7, 2020 "a specific part of the program" is a thread (amongst other parts) in your case, misunderstood your sentence there 🙂 Share this post Link to post
Uwe Raabe 2057 Posted October 7, 2020 Indeed, that could be misinterpreted easily. Although it is not necessarily a thread, but could as well be collecting data for a report or consolidate data monthly or other areas where multiple DB actions can be grouped. Obviously it is data is processed or retrieved in the background, where multi-threading steps in. Due to the integrated connection pooling, opening and closing a TFDConnection doesn't cost as much as it did in the past (BDE era that is). Share this post Link to post