Firebird. Cannot reconnect connection when disconnected by server

[eversun]

Now I am back again. Seems the main problem is somewhere in transaction handling.
If I run exactly the same code as I posted before - it works. But once I add forced transaction start and commit, it falls.
Here's the updated code:

...

Connection.Connect;

Connection.StartTransaction;
Query.SQL.Text := 'select * from RDB$DATABASE';
Query.Open;
Query.Close;
Connection.Commit;

Writeln('Now drop connection and press enter');
// Here I run command: DELETE FROM MON$ATTACHMENTS
Readln;

try
Connection.StartTransaction;
Query.Open;
Connection.Commit;
except
on E: Exception do Writeln('inner ', E.ClassName, ': ', E.Message);
end;

Writeln('Reconnecting');
Connection.Reconnect;
// This code is not reached, message "connection shutdown" with GDS code 335544856
// And actually I got a final exception here:
// Code: -902 Message: select * from RDB$DATABASE
// It somewhy is trying to unprepare prepared stamement
Writeln('Reconnecting done');

Connection.StartTransaction;
Query.Open;
Query.Close;
Connection.Commit;

[eversun]

What is really interesting, I see the following - try-except is commented here
procedure TZAbstractDbcConnection.CloseRegisteredStatements;
var I: Integer;
begin
for i := fRegisteredStatements.Count-1 downto 0 do begin
//try
IZStatement(fRegisteredStatements).Close;
//except end;
end;
end;

Sample code I had shown before is much simplified. I my real code after a set of unsuccesfull reconnects I finally got an AV right at function TZAbstractRODataset.CreateStatement(const SQL: string; Properties: TStrings)

if FTransaction <> nil
then Txn := THackTransaction(FTransaction).GetIZTransaction
else Txn := FConnection.DbcConnection.GetConnectionTransaction;
TxnCon := Txn.GetConnection; // << problem is here, as finally I got TxnCon = nil

[marsupilami]

Hello eversun,

I didn't have time to try to reproduce the issue. But maybe I can do some comment on the code that you mentioned.

What is really interesting, I see the following - try-except is commented here

Code: Select all

procedure TZAbstractDbcConnection.CloseRegisteredStatements;
var I: Integer;
begin
  for i := fRegisteredStatements.Count-1 downto 0 do begin
    //try
      IZStatement(fRegisteredStatements[i]).Close;
    //except end;
  end;
end;

Not having a try ... except here makes sense because otherwise error messages from the database will be silently ignored and the Zeos end user will never know why his application behaves in a strange way. Usually we think it is better to raise exceptions.

Best regards,

Jan

[eversun]

Possibly this function can be called in different situations ?
But in this case the problem is - call to reconnect causes an exception and does not produce the result expected

[marsupilami]

Possibly this function can be called in different situations ?

Yes - that is possible. I assume, it also gets called in the cleanup process after an connection is lost.

But in this case the problem is - call to reconnect causes an exception and does not produce the result expected

And I agree that this should not happen. Zeos should handle the exceptions in that context internally. But I doubt that this procedure is the right place to deal with the probnlem.

A side note for me: For constructing an automated test, it migt make sense to determine the current connection ID using the CURRENT_CONNECTION context variable (see here) and create a second connection for disconnecting the original connection. This way no user interaction would be required.

[aehimself]

A side note for me: For constructing an automated test, it migt make sense to determine the current connection ID using the CURRENT_CONNECTION context variable (see here) and create a second connection for disconnecting the original connection. This way no user interaction would be required.

If you manage to figure this out, you can also make a test for .AbortOperation. It was implemented a long-long time ago and I only tested it on MySQL, MSSQL (LibDB) and Oracle; rest of the implementations still can be faulty.

[marsupilami]

@eversun: I added some changes that should fix the problems that you reported. Could you please test the current SVN version?

edit: Some explanations:
The error code that gets generated when using "delete from mon$attachments" (isc_att_shutdown) was not yet on the list of error codes that make Zeos realize the connection was lost. Also Zeos tried to unprepare a statement on a lost connection which lead to another error.
The example code (see below) now works correctly on the legacy ISC API and it also works correctly on the interface based API in Delphi. In FPC it still generates an access violation. This might have to do with the different times at which FPC and Delphi free interfaces.

@aehimself:
my current solution looks like this:

Code: Select all

program FirebirdDisconnect;

{$APPTYPE CONSOLE}

{$R *.res}

uses
  SysUtils,
  ZConnection,
  ZDataSet,
  ZDbcInterbase6Utils, zcomponent;

var
  Connection: TZConnection;
  Query: TZQuery;
  ConnectionID: Integer;

procedure configConnection(Connection: TZConnection);
begin
  Connection.Protocol := 'firebird';
  Connection.HostName := '127.0.0.1';
  Connection.Database := 'c:\program files (x86)\topsales\datenbank\topsales.fdb';
  Connection.User := 'SYSDBA';
  Connection.Password := 'masterkey';
  Connection.LibraryLocation := 'D:\Projekte\TopSales\additional\firebird-embedded-4.0.0\fbclient.dll';
  //Connection.Properties.Add('FirebirdAPI=legacy');
end;

procedure dropConnection;
var
  Connection: TZConnection;
begin
  Connection := TZConnection.Create(nil);
  try
    configConnection(Connection);
    Connection.Connect;
    Connection.ExecuteDirect('delete from MON$ATTACHMENTS where MON$ATTACHMENT_ID = ' + IntToStr(ConnectionID));
    Connection.Disconnect;
  finally
    FreeAndNil(Connection);
  end;
end;

begin
  try
    Connection := TZConnection.Create(nil);

    configConnection(Connection);

    Query := TZQuery.Create(nil);
    Query.Connection := Connection;

    Connection.Connect;

    Connection.StartTransaction;
    Query.SQL.Text := 'select CURRENT_CONNECTION from RDB$DATABASE';
    Query.Open;
    ConnectionID := Query.Fields[0].AsInteger;
    Query.Close;
    Connection.Commit;

    Writeln('Connection dropped. Get ready to debug and press enter.');
    dropConnection;
    Readln;

    try
      Connection.StartTransaction;
      Query.Open;
      Connection.Commit;
    except
      on E: Exception do
        Writeln('inner ', E.ClassName, ': ', E.Message);
    end;

    Writeln('Reconnecting');
    Connection.Reconnect;
    // This code is not reached, message "connection shutdown" with GDS code 335544856
    // And actually I got a final exception here:
    // Code: -902 Message: select * from RDB$DATABASE
    // It somewhy is trying to unprepare prepared stamement
    Writeln('Reconnecting done');

    Connection.StartTransaction;
    Query.Open;
    Query.Close;
    Connection.Commit;

  except
    on E: Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;

  Writeln('Execution finished.');
  Readln;
end.

This is quite simple. For testing of AbortOperation we would need a long running operation for each database and I would have to fire off a separate thread to stop that operation. For Firebird we could do that easily using a code block or a stored procedure. But for other databases I am not sure how to do that.

[brick08]

If I understand correctly, when the connection is lost, the following procedures are called:

2022-06-27_12-10-53.jpg

In procedure TZAbstractStatement.ReleaseImmediat value FClosed set in True. And when the procedure TZAbstractStatement.Close is called, it does not go to the line: FConnection.DeregisterStatement(Self);
And then when Connection.Reconnect, in line IZStatement(fRegisteredStatements).Close raise AV.
I suggest patch:

Code: Select all

Index: src/dbc/ZDbcStatement.pas
===================================================================
--- src/dbc/ZDbcStatement.pas	(revision 7817)
+++ src/dbc/ZDbcStatement.pas	(working copy)
@@ -1667,7 +1667,7 @@
 var ImmediatelyReleasable: IImmediatelyReleasable;
 begin
   if not FClosed then begin
-    FClosed := True;
+    //FClosed := True;
     if (FOpenResultSet <> nil) and Supports(IZResultSet(FOpenResultSet), IImmediatelyReleasable, ImmediatelyReleasable) and
        (ImmediatelyReleasable <> Sender) then
       ImmediatelyReleasable.ReleaseImmediat(Sender, AError);

And small offtop patch:

Code: Select all

Index: src/dbc/ZDbcDbLib.pas
===================================================================
--- src/dbc/ZDbcDbLib.pas	(revision 7817)
+++ src/dbc/ZDbcDbLib.pas	(working copy)
@@ -435,7 +435,7 @@
     if Pointer(RawTemp) <> nil then
       FPlainDriver.dbSetLApp(LoginRec, Pointer(RawTemp));
 
-    SetRawFromProperties(ConnProps_AppName);
+    SetRawFromProperties(ConnProps_Language); //fix to ConnProps_Language
     if Pointer(RawTemp) <> nil then
       FPlainDriver.dbSetLNatLang(LoginRec, Pointer(RawTemp));

[marsupilami]

Hello brick08,

If I understand correctly, when the connection is lost, the following procedures are called:
2022-06-27_12-10-53.jpg
In procedure TZAbstractStatement.ReleaseImmediat value FClosed set in True. And when the procedure TZAbstractStatement.Close is called, it does not go to the line: FConnection.DeregisterStatement(Self);
And then when Connection.Reconnect, in line IZStatement(fRegisteredStatements).Close raise AV.
I suggest patch:

Code: Select all

Index: src/dbc/ZDbcStatement.pas
===================================================================
--- src/dbc/ZDbcStatement.pas	(revision 7817)
+++ src/dbc/ZDbcStatement.pas	(working copy)
@@ -1667,7 +1667,7 @@
 var ImmediatelyReleasable: IImmediatelyReleasable;
 begin
   if not FClosed then begin
-    FClosed := True;
+    //FClosed := True;
     if (FOpenResultSet <> nil) and Supports(IZResultSet(FOpenResultSet), IImmediatelyReleasable, ImmediatelyReleasable) and
        (ImmediatelyReleasable <> Sender) then
       ImmediatelyReleasable.ReleaseImmediat(Sender, AError);

It seems strange to not set FClosed to true. Release Immediat gets called in an emergency situation where a statement should be set to a closed state in any case. This is from the documentation on ReleaseImeediat:

Releases all driver handles and set the object in a closed Zombi mode waiting for destruction. Each known supplementary object, supporting this interface, gets called too. This may be a recursive call from parant to childs or vice vera. So finally all resources to the servers are released. This method is triggered by a connecton loss. Don't use it by hand except you know what you are doing.

So I think it might make more sense to do the derigistration, that is done in .Close in .ReleaseImmediat too. The thing that confuses me a bit is all the reference counting magic in .Close. Usually I would just call .Close from ReleaseImmediat. But I assume that this could trigger problems in the drivers because they will override .Close to add their own code too.
Do you have a small test case (project + database definition) where your change helps? This would help me in debugging this and doing the necessary changes.

And small offtop patch:

Code: Select all

Index: src/dbc/ZDbcDbLib.pas
===================================================================
--- src/dbc/ZDbcDbLib.pas	(revision 7817)
+++ src/dbc/ZDbcDbLib.pas	(working copy)
@@ -435,7 +435,7 @@
     if Pointer(RawTemp) <> nil then
       FPlainDriver.dbSetLApp(LoginRec, Pointer(RawTemp));
 
-    SetRawFromProperties(ConnProps_AppName);
+    SetRawFromProperties(ConnProps_Language); //fix to ConnProps_Language
     if Pointer(RawTemp) <> nil then
       FPlainDriver.dbSetLNatLang(LoginRec, Pointer(RawTemp));

I applied this patch. Thank you