#1 2019-01-28 11:14:18

isa
Member
Registered: 2018-01-09
Posts: 22

Potential Bug in mORMotSQLite3 TSQLRestServerDB.TransactionBegin

Hi,

I think there is a bug in mORMotSQLite3 TSQLRestServerDB.TransactionBegin.

In that method, whenever inherited TransactionBegin(aTable,SessionID) is called, and the result is true,
that means that TSQLRest.fTransactionActiveSession is set to the SessionID of that transaction.

So the transaction session is successfully allocated to this very transaction.
then there is the following code:

try
  DB.TransactionBegin; // will call DB.Lock
except
  on ESQLite3Exception do
      result := false;
end;

So if DB.TransactionBegin were to throw an exception, the exception is caught returning a False, which indicates that running the transaction failed.
However, returning a False is not enough! Because earlier inherited TransactionBegin(aTable,SessionID) succeeded, TSQLRest.fTransactionActiveSession is still set to SessionID, and is NOT reset.
Meaning that whenever this situation occurs (that DB.TransactionBegin throws an exception), fTransactionActiveSession Remains > 0, meaning that NO other transaction can be start, because
inherited TransactionBegin(aTable,SessionID) (which is the first thing that is called whenever you call TransactionBegin on mORMotSQLite3) will always return false as a result.
It ONLY returns true, when fTransactionActiveSession=0.

So EVERY other TransactionBegin call on mORMotSQLite3 will fail if even once the DB.TransactionBegin call in the highlighted code above fails.

Correct me if I'm wrong but that's my conclusion.
Because if we reach the call DB.TransactionBegin, it means that inherited TransactionBegin(aTable,SessionID), meaning that fTransactionActiveSession and fTransactionTable are both allocated to that very call.
meaning that if this transaction fails, inherited rollback needs to be called to release back fTransactionTable and especially fTransactionActiveSession, because that blocks any other attempt to do a transaction on that database.

This issue can be avoided by doing this:

try
  TransactionBegin
  ...
  Commit
finally
  Rollback
end;

So by basically moving the TransactionBegin INSIDE the Try finally block, that even if TransactionBegin were to fail, and even if there is no actual transaction active, it still calls Rollback, which on its turn Calls the TSQLRest.Rollback in mORMot.pas, which resets fTransactionActiveSession back to 0.
This is a weird solution and shouldn't be necessary, because whenever something goes wrong in DB.TransactionBegin, it still Calls Execute('ROLLBACK TRANSACTION;'); So Me doing an extra rollback in that scenario shouldn't be necessary. And procedure wise doesn't make sense to call Rollback if BeginTransaction failed. Unless it was meant to be used like that which i doubt.
Also this code is not really safe, If TransactionBegin were to fail for other reasons, unless you always use a different sessionID's for each other transaction call you do, especially when done on other threads.

Nonetheless I think this would be a possible solution that would make more sense:
So the exact same code as above, but changed as following: (Note: this is not necessarily the solution I suggest, is just to give an idea)

try
  DB.TransactionBegin; // will call DB.Lock
except
  on ESQLite3Exception do
  begin
    result := false;
    try
      // resetting fTransactionActive flag
      inherited RollBack;
    Except
      //
    end;
  end;
end;

Once again, because we know fTransactionActiveSession is allocated to that specific call because of the succeeding inherited call, I think it should be safe to do an inherited Rollback call that releases back fTransactionActiveSession.
Once again, correct me if I'm wrong, or feel free to suggest better solutions if possible.



Now, of course there is a reason why I suggest this fix, because I have an issue as following:
Imagine 2 consecutively running threads (2 clients) that both tries to do a transaction at the same time. Useful to note that I always pass GetCurrentThreadId as SessionID.
Now this is the exact scenario that happens:

> Thread 1: BeginTransaction Enter
> Thread 1: BeginTransaction Exit (success)
> Thread 1: Commit Enter
> Thread 1: Execute('COMMIT TRANSACTION') (NOTE: It might be possible that this line takes longer, or is called after Thread 2 has started, but the important part is that it has proceeded enough so that at the point when Thread 2 calls Execute('ROLLBACK TRANSACTION;'), it throws an exception)
> Thread 1: fTransactionActiveSession := 0
> Thread 2: BeginTransaction Enter (Initial check succeeds, because inherited BeginTransaction returns true because fTransactionActiveSession = 0) (IMPORTANT NOTE!!: At this point Thread 1 has NOT set fTransactionActive back to False yet in SynSQLite3 in TSQLDataBase)
> Thread 2: TSQLDataBase.TransactionBegin ENTER
> Thread 2: TSQLDataBase.TransactionBegin Throws Exception: Because fTransactionActive is still true from Thread 1, so instead this code is called by Thread 2: Execute('ROLLBACK TRANSACTION;'); which throws an exception because there is no Running transaction as Thread 1 already called Execute('COMMIT TRANSACTION')
                  the exception: ESQLite3Exception {"ErrorCode":1,"SQLite3ErrorCode":2,"Message":"Error SQLITE_ERROR (1) using 3.8.7.4 - 'cannot rollback - no transaction is active' extended_errcode=1"}
> Thread 2: Sets fTransactionActive to False in TSQLDataBase.TransactionBegin
> Thread 2: BeginTransaction Exit (while fTransactionActiveSession is still set to Thread 2)
> Thread 1: Commit Exit. (doesn't matter whether or not Thread 2 or 1 exits first)

So as you can see in that example, fTransactionActive (in SynSQLite3 in TSQLDataBase) is set to false too late after fTransactionActiveSession (in mORMot in TSQLRest) is set to 0 by Thread 1, causing TransactionBegin of Thread 2 to proceed further than it should be, causing it to throw an exception and leaving the transaction locked because it doesn't reset fTransactionActiveSession after an exception was thrown.
fTransactionActive (in SynSQLite3 in TSQLDataBase) is in this case the culprit, but that doesn't mean that TransactionBegin should be less safe, because other code is in the wrong. Nevertheless also it does seem to me like fTransactionActive might not be thread safe, and am not sure if that might be related to this issue.


Hope my explanation makes sense and isn't too complicated, can add more details if necessary. Or explain further.

NOTE: The exact order of the calls between the two threads might not be 100% correct, but it's more or less in that order, hopefully making my issue clear.

Last edited by isa (2019-01-28 14:00:10)

Offline

#2 2019-01-28 14:30:04

isa
Member
Registered: 2018-01-09
Posts: 22

Re: Potential Bug in mORMotSQLite3 TSQLRestServerDB.TransactionBegin

I just noticed on the definition of TransactionBegin the following comment:
// - must be aborted with Rollback if any SQL statement failed

Which is basically what I end up doing in my "bugfix" proposal.
Reading this comment, how am I supposed to write correctly a proper transaction clause?
As this does not comply with that comment:

TransactionBegin
try
  ...
  Commit
finally
  Rollback
end;

Any SQL Exception is caught INSIDE TransactionBegin, so there is no way for us to tell whether or not an exception occurred, and if or not this is an exception as a result of a failed SQL statement.

EDIT: NOTE: I forgot to mention. That in my code whenever TransactionBegin fails, I stop that code, as I don't want to proceed doing actions on the database, if the transaction hasn't started yet, nor do I want to call rollback, in case transactionBegin failed, because another transaction was in progress. So I either need specific details as to why the TransactionBegin Failed, or upon failure, transactionBegin should do a proper fallback depending on why it failed.

Last edited by isa (2019-01-29 09:20:43)

Offline

#3 2019-01-29 10:18:24

pvn0
Member
From: Slovenia
Registered: 2018-02-12
Posts: 210

Re: Potential Bug in mORMotSQLite3 TSQLRestServerDB.TransactionBegin

It took me a while but I see your point, you are correct, there is a chance that TransactionBegin will call Execute('ROLLBACK TRANSACTION;') and throw exception just because fTransactionActive is not actually thread safe.

However, this seems to be accounted for, if you look at TSQLRestServerDB.TransactionBegin :

function TSQLRestServerDB.TransactionBegin(aTable: TSQLRecordClass; SessionID: cardinal=1): boolean;
begin
  result := inherited TransactionBegin(aTable,SessionID);
  if not result then
    exit; // fTransactionActive flag was already set
  try
    DB.TransactionBegin; // will call DB.Lock
  except
    on ESQLite3Exception do
      result := false;
  end;
end;

In your example, DB.TransactionBegin will throw exception and it's caught and result is set to false.
You can inherit from Server classes so you can fine tune this function yourself to return more info if you need.

Personally I think the transaction code is very clever, I like how TransactionActiveSession is actually released at the TSQLRest level before any transaction commit or rollback actually takes place, so that in a multi-thread env, threads don't need to wait until Database execution has already happened but instead are automatically blocked through a critical lock when they try to Commit.

Last edited by pvn0 (2019-01-29 10:25:20)

Offline

#4 2019-01-29 11:01:19

isa
Member
Registered: 2018-01-09
Posts: 22

Re: Potential Bug in mORMotSQLite3 TSQLRestServerDB.TransactionBegin

pvn0 wrote:

It took me a while but I see your point, you are correct, there is a chance that TransactionBegin will call

Execute('ROLLBACK TRANSACTION;')

and throw exception just because fTransactionActive is not actually thread safe.

However, this seems to be accounted for, if you look at TSQLRestServerDB.TransactionBegin :

function TSQLRestServerDB.TransactionBegin(aTable: TSQLRecordClass; SessionID: cardinal=1): boolean;
begin
  result := inherited TransactionBegin(aTable,SessionID);
  if not result then
    exit; // fTransactionActive flag was already set
  try
    DB.TransactionBegin; // will call DB.Lock
  except
    on ESQLite3Exception do
      result := false;
  end;
end;

In your example, DB.TransactionBegin will throw exception and it's caught and result is set to false.
You can inherit from Server classes so you can fine tune this function yourself to return more info if you need.

Personally I think the transaction code is very clever, I like how ActiveTransactionSession is actually released at the TSQLRest level before any transaction commit or rollback actually takes place, so that in a multi-thread env, threads don't need to wait until Database execution has already happened but instead are automatically blocked through a critical lock when they try to Commit.

First of all, thank you for your reply.
I have no problem with how fActiveTransactionSession is released with a commit.
On a commit there are 3 things that needs to be done for another transaction to be able to starts which is in this order:

fActiveTransactionSession := 0;            // this is done thread safe
Execute('COMMIT TRANSACTION;');      // This is the actual release on the SQL database
fTransactionActive := False;                  // This one is set to false at the very last moment in case of a commit, yet on a transaction begin this is the last check that is done before another transaction starts (not necessarily a bad thing), and it seems like it's not thread safe.

Like you mentioned, this is overall good, it works perfectly as intended 99% of the time, if not more. Thing is, I have a rather large mORMot server with lots of users on it. so it can happen like once or twice a week that I have this bug.
I mean it's even stated as comments on the TransactionBegin definition, that a rollback has to be done in case of a sql execution error.

The SQL Execution error does happen in my case because of not being fully thread safe. Not a huge deal, it has proper error handling, so nothing goes wrong, apart from the part that rollback is not called.
And with rollback I mean the rollback method of TSQLRest.RollBack in mORMot.pas, though a full rollback call from TSQLRestServerDB.RollBack should also do the trick of course.

Perhaps this rollback is not done, because not in all cases the SQL Error is due to there not being anything that can be rolled back.
Perhaps Arnaud decided to do this, just to make sure that in case the actual rollback on the SQL failed (meaning that the transaction is still active), he wants to avoid calling TSQLRest.RollBack as it is not a wanted scenario. So basically this not necessarily being a bug but a decision.

As you saw the TransactionBegin does an Execute('ROLLBACK TRANSACTION;') in TSQLDataBase.TransactionBegin in case fTransactionActive is True. If you ask me, If this happens, clearly a rollback is called on SQL level. so it should also be called on mORMot level on TSQLRest. That's the inconsistency here.
So if fTransactionActive is True,
Execute('ROLLBACK TRANSACTION;') is called, which is good
fTransactionActive is set to False, which is also good
But fActiveTransactionSession remains what it is. So in that Rollback scenario that happens inside TransactionBegin, in some way also TSQLRest.RollBack in mORMot.pas has to be called. so that TSQLRest in mORMot.pas and TSQLDataBase in SynSQLite3 are in sync.

And once fActiveTransactionSession remains > 0 while fTransactionActive is set to false, you cannot start a transaction anymore whatsoever unless you do a manual Full Rollback first.
And my issue was even worse. I didn't used to use a separate SessionID's for my transactions until a couple months back. Causing journal files to be created after this bug happened, and every single change on the database that happens outside transactions would be stored in those journals, and I had no manual code to call commit, and I also wasn't sure back then if this transaction bug was caused due to some error in the code, thus me not necessarily wanting to commit but do a rollback instead, but that also meant that I would loose all those other changes stored in those journal files. Causing lots of issues with our users.

Luckily the journal file issues are gone ever since using the thread ID of each separate client connection as the session ID, but still this bug happens from time to time, blocking any other transaction to run from that point on until I restart the running REST server.

So yeah, not certain what possible solution I'd go for this, as I also don't want to possibly introduce other unwanted behavior as a result of my changes and me potentially overlooking thins.

Last edited by isa (2019-01-29 11:01:42)

Offline

#5 2019-01-30 10:42:27

pvn0
Member
From: Slovenia
Registered: 2018-02-12
Posts: 210

Re: Potential Bug in mORMotSQLite3 TSQLRestServerDB.TransactionBegin

OK, I've created a minimal working example to demonstrate that this bug is legit, @isa please confirm this is the same problem you have.

This example creates (n) number of async tasks that start their own transactions, each task also sleeps anywhere from 0-5000 ms before trying to create a transaction to better simulate real life workload. Like @isa explained, eventually the code that isn't thread safe will trip over itself and from that point on , no transactions can be done at all.
9CXx68X.png

source code below, using https://github.com/gabr42/OmniThreadLibrary for threading so get that if you don't have it yet. Compiled and tested with Delphi XE7 64bit.

program Project1;

uses
  Vcl.Forms,
  Unit1 in 'Unit1.pas' {Form1};

{$R *.res}

begin
  Application.Initialize;
  Application.MainFormOnTaskbar := True;
  Application.CreateForm(TMainForm, MainForm);
  Application.Run;
end.
object MainForm: TMainForm
  Left = 0
  Top = 0
  Caption = 'Test'
  ClientHeight = 399
  ClientWidth = 562
  Color = clBtnFace
  Font.Charset = DEFAULT_CHARSET
  Font.Color = clWindowText
  Font.Height = -11
  Font.Name = 'Tahoma'
  Font.Style = []
  OldCreateOrder = False
  OnCreate = FormCreate
  PixelsPerInch = 96
  TextHeight = 13
  object lb: TListBox
    Left = 0
    Top = 64
    Width = 562
    Height = 335
    Align = alBottom
    ItemHeight = 13
    TabOrder = 0
  end
  object bRunMany: TButton
    Left = 16
    Top = 16
    Width = 75
    Height = 25
    Caption = 'Run'
    TabOrder = 1
    OnClick = bRunManyClick
  end
  object editMany: TEdit
    Left = 104
    Top = 18
    Width = 89
    Height = 21
    NumbersOnly = True
    TabOrder = 2
    Text = '256'
  end
  object bRunSingle: TButton
    Left = 464
    Top = 16
    Width = 75
    Height = 25
    Caption = 'Run single'
    TabOrder = 3
    OnClick = bRunSingleClick
  end
end
unit Unit1;

interface

uses
  Winapi.Windows,
  Winapi.Messages,
  System.SysUtils,
  System.Variants,
  System.Classes,
  Vcl.Graphics,
  Vcl.Controls,
  Vcl.Forms,
  Vcl.StdCtrls,
  SynCommons,
  Synsqlite3Static,
  mORMot,
  mORMotSQLite3,
  OTLComm,
  OtlTask,
  OtlParallel,
  OtlTaskControl;

const
  MSG_LOG = WM_APP + 1;

type
  TSQLTest = class(TSQLRecord)
  private
    FA, FB, FC, FD, FE: RawUTF8;
  published
    property A: RawUTF8 read FA write FA;
    property B: RawUTF8 read FB write FB;
    property C: RawUTF8 read FC write FC;
    property D: RawUTF8 read FD write FD;
    property E: RawUTF8 read FE write FE;
  end;

  TMainForm = class(TForm)
    lb: TListBox;
    bRunMany: TButton;
    editMany: TEdit;
    bRunSingle: TButton;
    procedure FormCreate(Sender: TObject);
    procedure bRunManyClick(Sender: TObject);
    procedure bRunSingleClick(Sender: TObject);
  private
    fModel: TSQLModel;
    fRestServer: TSQLRestServerDB;
  protected
    procedure AddRecords(const task: IOmniTask);
    procedure OnTaskMessage(const task: IOmniTaskControl; const msg: TOmniMessage);
    procedure OnTaskTerminated(const task: IOmniTaskControl);
  end;

var
  MainForm: TMainForm;

implementation

{$R *.dfm}

procedure TMainForm.AddRecords(const task: IOmniTask);
var
  I: Integer;
  R: TSQLTest;
  taskID: Cardinal;
  lLength: Integer;
  lArray: TArray<TSQLTest>;
begin
  Randomize;
  Sleep(Random(5000));
  if task.CancellationToken.IsSignalled then
    exit;
  taskID := GetCurrentThreadId;
  Randomize;
  lLength := Random(256);
  SetLength(lArray, lLength);
  try
    for I := 0 to lLength - 1 do
    begin
      R := TSQLTest.Create;
      R.A := SynCommons.StringToUTF8(TGUID.NewGuid.ToString);
      R.B := SynCommons.StringToUTF8(TGUID.NewGuid.ToString);
      R.C := SynCommons.StringToUTF8(TGUID.NewGuid.ToString);
      R.D := SynCommons.StringToUTF8(TGUID.NewGuid.ToString);
      R.E := SynCommons.StringToUTF8(TGUID.NewGuid.ToString);
      lArray[I] := R;
    end;

    task.Comm.Send(MSG_LOG, Format('task(%d) starting transaction', [taskID]));
    if fRestServer.TransactionBegin(TSQLTest, taskID) then
    begin
      try
        task.Comm.Send(MSG_LOG, Format('task(%d) Transaction started', [taskID]));
        // .... modify the database content, raise exceptions on error
        for I := 0 to lLength - 1 do
          fRestServer.Add(lArray[I], true);
        task.Comm.Send(MSG_LOG, Format('task(%d) Trying to Commit ...', [taskID]));
        fRestServer.Commit(taskID, true);
        task.Comm.Send(MSG_LOG, Format('task(%d) Commit Success!', [taskID]));
      except
        on E: Exception do
        begin
          task.Comm.Send(MSG_LOG, Format('task(%d) Performing RollBack, EXCEPTION : %s', [taskID, E.Message]));
          fRestServer.RollBack(taskID); // in case of error
        end;
      end;
    end
    else
      task.Comm.Send(MSG_LOG, Format('task(%d) could not start Transaction', [taskID]));

  finally
    for I := 0 to lLength - 1 do
      lArray[I].Free;
  end;

end;

procedure TMainForm.bRunManyClick(Sender: TObject);
var
  I: Integer;
begin
  for I := 0 to StrToInt(editMany.Text) - 1 do
    Parallel.Async(AddRecords, Parallel.TaskConfig.OnMessage(OnTaskMessage).OnTerminated(OnTaskTerminated));
end;

procedure TMainForm.bRunSingleClick(Sender: TObject);
begin
  Parallel.Async(AddRecords, Parallel.TaskConfig.OnMessage(OnTaskMessage).OnTerminated(OnTaskTerminated));
end;

procedure TMainForm.FormCreate(Sender: TObject);
begin
  fModel := TSQLModel.Create([TSQLTest]);
  fRestServer := TSQLRestServerDB.Create(fModel, ExeVersion.ProgramFilePath + 'test.db');
  fModel.Owner := fRestServer;
  fRestServer.CreateMissingTables;
end;

procedure TMainForm.OnTaskMessage(const task: IOmniTaskControl; const msg: TOmniMessage);
begin
  if msg.MsgID = MSG_LOG then
    lb.Items.Add(msg.MsgData.AsString);
end;

procedure TMainForm.OnTaskTerminated(const task: IOmniTaskControl);
var
  lException: Exception;
begin
  if not Application.Terminated then
    if Assigned(task.FatalException) then
    begin
      lException := task.DetachException;
      try
        lb.Items.Add(lException.Message);
      finally
        lException.Free;
      end;
    end;
end;

end.

Last edited by pvn0 (2019-01-30 10:56:10)

Offline

#6 2019-01-30 11:04:42

isa
Member
Registered: 2018-01-09
Posts: 22

Re: Potential Bug in mORMotSQLite3 TSQLRestServerDB.TransactionBegin

Indeed, this seems exactly like the issue. Especially if between the Commit succes and first Transaction Start failure an exception took place of this type: 'cannot rollback - no transaction is active'.
Thank you very much for your time and effort!

The solution I did was the following, inside the TransactionBegin in mORMotSQLite3.pas:

function TSQLRestServerDB.TransactionBegin(aTable: TSQLRecordClass; SessionID: cardinal=1): boolean;
begin
  result := inherited TransactionBegin(aTable,SessionID);
  if not result then
    exit; // fTransactionActive flag was already set
  try
    DB.TransactionBegin; // will call DB.Lock
  except
    on ESQLite3Exception do
    begin
      result := false;
      try
        // releasing fTransactionActive flag
        inherited RollBack(SessionID);
      Except
        //
      end;
    end;
  end;
end;

Now this seems a little uglyish to have a try except in a try except, so I also had this as a solution:

function TSQLRestServerDB.TransactionBegin(aTable: TSQLRecordClass; SessionID: cardinal=1): boolean;
begin
  result := inherited TransactionBegin(aTable,SessionID);
  if not result then
    exit; // fTransactionActive flag was already set
  try
    try
      DB.TransactionBegin; // will call DB.Lock
    finally
        if NOT DB.TransactionActive then
          inherited RollBack(SessionID);
    end
  except
    on ESQLite3Exception do
      result := false;
  end;
end;

Now that 2nd one should also be okay, as the only scenario where DB.TransactionBegin ends with fTransactionActive remaining false, is when the call Execute('ROLLBACK TRANSACTION') throws an exception, which is caught by the try except.
So both solutions should be fine, but I did prefer the 1st solution, because me not being able to fully trust fTransactionActive being thread safe, especially when SessionID's are not unique per transaction.

Once again thank you for your efforts. Really appreciate that.

Since you already have that code, is it also possible for you to test whether or not the issue is actually solved after adding the fix I mentioned to mORMotSQLite3.pas?

Offline

#7 2019-01-30 11:28:49

pvn0
Member
From: Slovenia
Registered: 2018-02-12
Posts: 210

Re: Potential Bug in mORMotSQLite3 TSQLRestServerDB.TransactionBegin

@isa , your 1st solution fixes the problem, nice work! smile @ab please review this.

Offline

#8 2019-01-30 11:45:12

isa
Member
Registered: 2018-01-09
Posts: 22

Re: Potential Bug in mORMotSQLite3 TSQLRestServerDB.TransactionBegin

pvn0 wrote:

@isa , your 1st solution fixes the problem, nice work! smile @ab please review this.

Thank you for the the confirmation, and once again thank you for replying to this post and taking your time and effort to look this up.
Really appreciate that!

Thank you.

Offline

#9 2019-01-30 13:37:19

pvn0
Member
From: Slovenia
Registered: 2018-02-12
Posts: 210

Re: Potential Bug in mORMotSQLite3 TSQLRestServerDB.TransactionBegin

Exceptions can be very expensive tho, another approach would be to read/write your own crit.locked boolean value whenever you do start/finish transaction on server side. That way you will always 100% know if transaction is active or not.

Best regards.

Offline

#10 2019-02-21 14:17:01

isa
Member
Registered: 2018-01-09
Posts: 22

Re: Potential Bug in mORMotSQLite3 TSQLRestServerDB.TransactionBegin

@ab
I have been using my fix for a while now, and my issues are solved.
Could you perhaps look into this? Is there perhaps a reason you don't want to change this? Or do you perhaps have other plans with mORMot, now that you switched to AI development?

Offline

#11 2019-04-26 12:40:30

pvn0
Member
From: Slovenia
Registered: 2018-02-12
Posts: 210

Re: Potential Bug in mORMotSQLite3 TSQLRestServerDB.TransactionBegin

@ab, @isa,

Please see #191 pull request. I've tested and I believe this is best fix.

Offline

#12 2019-04-26 12:59:29

isa
Member
Registered: 2018-01-09
Posts: 22

Re: Potential Bug in mORMotSQLite3 TSQLRestServerDB.TransactionBegin

pvn0 wrote:

@ab, @isa,

Please see #191 pull request. I've tested and I believe this is best fix.

Thank you for sharing your attempt on fixing it.
However that does not work for my needs.
In your fix you simply check the result of the "DB.TransactionBegin"  call alongside the existing result check of  "inherited TransactionBegin(aTable,SessionID)"


Problem is that FB.TransactionBegin Doesn't simply return False, but raises an Exception. So your check wouldn't make any difference in my scenario. Still leaving the Transaction at mormot level not rolled back.

Once again my solution was to change the Code in the Except block to:

except
  on ESQLite3Exception do
  begin
    result := false;
    try
      // releasing fTransactionActive flag
      inherited RollBack(SessionID);
    Except
      //
    end;
  end;
end;

As I did state above. This works good enough for me. Not saying this is the best solution, but your solution unfortunately doesn't fix my issue.

Last edited by isa (2019-04-26 13:00:40)

Offline

#13 2019-04-26 13:36:49

pvn0
Member
From: Slovenia
Registered: 2018-02-12
Posts: 210

Re: Potential Bug in mORMotSQLite3 TSQLRestServerDB.TransactionBegin

I am happy your solution works for you, however I think you have misread the fix. The reason you get exception in DB.TransactionBegin is because the race condition creates a soft lock always thinking there is an active transaction when in reality there is not and as a result the db sqlite code will throw exception in something like Exception: "no transaction active" when the TransactionBegin tries to rollback a previous transaction that is no longer active. 

Either way, I have tested this fix with the sample program I provided in this thread and it solves this problem.

Offline

#14 2019-04-26 14:13:18

isa
Member
Registered: 2018-01-09
Posts: 22

Re: Potential Bug in mORMotSQLite3 TSQLRestServerDB.TransactionBegin

pvn0 wrote:

I am happy your solution works for you, however I think you have misread the fix. The reason you get exception in DB.TransactionBegin is because the race condition creates a soft lock always thinking there is an active transaction when in reality there is not and as a result the db sqlite code will throw exception in something like Exception: "no transaction active" when the TransactionBegin tries to rollback a previous transaction that is no longer active. 

Either way, I have tested this fix with the sample program I provided in this thread and it solves this problem.


My bad, indeed you call DB.BeingTransaction BEFORE Inherited transaction.
Thanks for your input.

Offline

Board footer

Powered by FluxBB