BookmarkValid problems

[Traptak]

Hello,

I have problems with TMemDataSet.BookmarkValid function. Sometimes it returns wrong values but sometimes cases it raises Access Violation exception. Earlier I have used version 7.1.0 and there everything worked correctly. I have check sources and in version 7.1.0 bookmark validating code is:

Code: Select all

  if IntPtr(Bookmark) <> nil then
    Result := (Bookmark.Order <> -1) or (IntPtr(Bookmark.Item) <> nil)
  else
    Result := False;

  if Result and Filtered then 
    Result := not OmitRecord(Bookmark.Item);

but in version 9.2.5 code is:

Code: Select all

  if IntPtr(Bookmark) <> nil then
    Result := ((Bookmark.Order <> -1) and (Bookmark.Order <= FRecordCount)) or
      ((Bookmark.RefreshIteration = FRefreshIteration) and (IntPtr(Bookmark.Item) <> nil) and (Bookmark.Item.Order <= FRecordCount))
  else
    Result := False;

  if Result and Filtered then
    Result := not OmitRecord(Bookmark.Item);

Here are few conditions was added and I think here is a problem.

How to reproduce problem? Create the simplest table in Oracle:

Code: Select all

create table T_TEST
(
  test_id INTEGER
)

Then insert one row to the table:

Code: Select all

insert into t_test values(1)

Test application is dfm file:

Code: Select all

object Form1: TForm1
  Left = 0
  Top = 0
  Caption = 'Form1'
  ClientHeight = 449
  ClientWidth = 745
  Color = clBtnFace
  Font.Charset = DEFAULT_CHARSET
  Font.Color = clWindowText
  Font.Height = -11
  Font.Name = 'Tahoma'
  Font.Style = []
  OldCreateOrder = False
  PixelsPerInch = 96
  TextHeight = 13
  object dbg1: TDBGrid
    Left = 0
    Top = 0
    Width = 745
    Height = 408
    Align = alClient
    DataSource = DataSource1
    Options = [dgEditing, dgTitles, dgIndicator, dgColumnResize, dgColLines, dgRowLines, dgTabs, dgConfirmDelete, dgCancelOnExit, dgMultiSelect, dgTitleClick, dgTitleHotTrack]
    TabOrder = 0
    TitleFont.Charset = DEFAULT_CHARSET
    TitleFont.Color = clWindowText
    TitleFont.Height = -11
    TitleFont.Name = 'Tahoma'
    TitleFont.Style = []
  end
  object pnl1: TPanel
    Left = 0
    Top = 408
    Width = 745
    Height = 41
    Align = alBottom
    Caption = 'pnl1'
    TabOrder = 1
    object lblError: TLabel
      Left = 512
      Top = 8
      Width = 3
      Height = 13
      Font.Charset = DEFAULT_CHARSET
      Font.Color = clRed
      Font.Height = -11
      Font.Name = 'Tahoma'
      Font.Style = []
      ParentFont = False
    end
    object Button1: TButton
      Left = 9
      Top = 6
      Width = 75
      Height = 25
      Caption = 'Problem'
      TabOrder = 0
      OnClick = Button1Click
    end
  end
  object OraSession: TOraSession
    Username = 'system'
    Server = 'xe'
    LoginPrompt = False
    Left = 224
    Top = 184
    EncryptedPassword = '98FF96FF9BFF9AFF87FF'
  end
  object OraQuery1: TOraQuery
    Session = OraSession
    SQL.Strings = (
      'select * from t_test where test_id = 1')
    CachedUpdates = True
    Options.StrictUpdate = False
    Options.PrepareUpdateSQL = True
    Left = 320
    Top = 200
  end
  object DataSource1: TDataSource
    DataSet = OraQuery1
    OnDataChange = DataSource1DataChange
    Left = 176
    Top = 104
  end
  object OraSQL1: TOraSQL
    Session = OraSession
    SQL.Strings = (
      'delete from t_test where test_id = 1')
    Left = 400
    Top = 248
  end
end

pas file:

Code: Select all

unit Unit1;

interface

uses
  Winapi.Windows, Winapi.Messages, System.SysUtils, System.Variants, System.Classes, Vcl.Graphics,
  Vcl.Controls, Vcl.Forms, Vcl.Dialogs, Data.DB, MemDS, DBAccess, Ora, OraCall, Vcl.Grids, Vcl.DBGrids, Vcl.ExtCtrls,
  Vcl.StdCtrls, DASQLMonitor, OraSQLMonitor;

type
  TForm1 = class(TForm)
    OraSession: TOraSession;
    OraQuery1: TOraQuery;
    dbg1: TDBGrid;
    DataSource1: TDataSource;
    pnl1: TPanel;
    Button1: TButton;
    OraSQL1: TOraSQL;
    lblError: TLabel;
    procedure Button1Click(Sender: TObject);
    procedure DataSource1DataChange(Sender: TObject; Field: TField);
  private
    { Private declarations }
  public
    { Public declarations }
  end;

var
  Form1: TForm1;

implementation

{$R *.dfm}

procedure TForm1.Button1Click(Sender: TObject);
begin
  OraSession.Connect;
  OraQuery1.Open;
  dbg1.SelectedRows.CurrentRowSelected := True;
  OraQuery1.DisableControls;
  try
    OraSQL1.Execute;
  finally
    OraQuery1.EnableControls;
  end;
    OraQuery1.RefreshRecord;
  OraSession.Rollback;  // For testing only - we don't row deleting
  try
    if OraQuery1.BookmarkValid(dbg1.SelectedRows.Items[0]) then
      raise Exception.Create('Bookmark valid, for non existing record.');
  finally
    OraSession.Disconnect;
  end;
end;

procedure TForm1.DataSource1DataChange(Sender: TObject; Field: TField);
begin
  if dbg1.SelectedRows.Count > 0 then
    if not OraQuery1.BookmarkValid(dbg1.SelectedRows[0]) then
      dbg1.SelectedRows.Clear
    else
      lblError.Caption := 'Bookmark valid for non existing record';
end;
end.

Application is written in DXE4 pro. After compiling press Problem button. Now you can get one of two kind of results:
1. Exception "Bookmark valid for non existing record" will be raised
2. Exception Access Violation is raised.
Kind of result depends on memory value which will be used to the bookmark creation. Sometime it will be wrong result sometime it will be Access Violation.

best regards Adam Siwon

[AlexP]

Hello,

Your code works correctly
1) After execution of EnableControls, the record still exists in the DataSet, therefore the

Code: Select all

not OraQuery1.BookmarkValid(dbg1.SelectedRows[0]) 

condition in the DataSource1DataChange method will return False, and the

Code: Select all

"Bookmark valid for non existing record" 

message is displayed.
2) After execution of the RefreshRecord method,

Code: Select all

dbg1.SelectedRows.Count 

will still be greater than 0, however, the record doesn't exist anymore, therefore the

Code: Select all

dbg1.SelectedRows.Clear method

will be called.
3) After that, on an attempt to access

Code: Select all

dbg1.SelectedRows.Items[0] 

(and since the Clear method was called at the previous step), you will get an AV on an attempt to access a non-existing object.

[Traptak]

Hello,

I found this problem, but to the forum I have paste wrong version of code - my mistake. Final version of test procedure should be:

Code: Select all

procedure TForm1.Button1Click(Sender: TObject);
begin
  OraSession.Connect;
  OraQuery1.Open;
  dbg1.SelectedRows.CurrentRowSelected := True;
  OraQuery1.DisableControls;
  try
    OraSQL1.Execute;
  finally
    OraQuery1.EnableControls;
  end;
    OraQuery1.RefreshRecord;
  OraSession.Rollback;  // For testing only - we don't row deleting
  try
    if (dbg1.SelectedRows.Count > 0) and OraQuery1.BookmarkValid(dbg1.SelectedRows.Items[0]) then
      raise Exception.Create('Bookmark valid, for non existing record.');
  finally
    OraSession.Disconnect;
  end;
end;

procedure TForm1.DataSource1DataChange(Sender: TObject; Field: TField);
begin
  if dbg1.SelectedRows.Count > 0 then
    if not OraQuery1.BookmarkValid(dbg1.SelectedRows[0]) then
      dbg1.SelectedRows.Clear
    else
      if OraQuery1.RecordCount = 0 then
        lblError.Caption := 'Bookmark valid for non existing record';
end;

So now there is no exception, but I think in this example ODAC still don't works correctly. DataSource1DataChange is called twice. First time after calling EnableControls and second time after calling RefreshRecord. In the second call OraQuery1 is empty but BookmarkValid still returns value True. Its because in parameter Bookmark in procedure TMemData.BookmarkValid is set to reference to memory which was freed in record refreshing process. (TBookmark value is converted to the PRecBookmark in TMemDataSet.BookmarkValid procedure without any pointer check)
Sometimes it is random memory space - this give wrong results but sometime it is nonexistent memory space - this give AV exception.

As you can see here is call stack for RefreshRecord procedure.
System._FreeMem($7EEC9C30)
MemData.TBlockManager.FreeBlock($7EEC9C30)
MemData.TBlockManager.FreeItem($7EEC9C3C)
MemData.TMemData.DeleteItem($7EEC9C3C)
MemData.TMemData.RemoveRecord
DBAccess.TCustomDADataSet.RefreshRecord
Unit1.TForm1.Button1Click($7EF2FF70)
Memory block with address $7EEC9C30 is freed. And if you check the call TMemData.BookmarkValid from second call DataSource1DataChange you can see, that Bookmark.Item still points to the freed memory space ($7EEC9C30 in my case). Maybe in almost all cases memory will not be damaged, but I think this is not good solution. This problem is good visible when you use FastMM4 memory manager. I'm using it, so maybe this make that problem occurs in my project everytime.

best regards Adam Siwon

[AlexP]

Hello,

After code modification, your project works correctly:
1) After calling EnableControls, the number of records is 1, and Bookmark is valid, therefore nothing happens.
2) After calling refresh, the number of records is 0, and Bookmark is not valid, therefore the Clear method is called.
3) The standard Delphi methods, TDataSet.EnableControls and TDataSet.Resync (which is called in the Refresh method), call the deDataSetChange event, that causes calling of onDataChange

[Traptak]

Hello Alex,

sorry for the delay, but I had no time recently. I have read your answer and I'm affraid you are wrong and the bug still exist in ODAC. I think you make test only for happy flow, which is dangerous and it give you false-positive results.
Last time I wrote you about memory freeing. After your answer I see you don't read this. So I made some changes to my example. Now problem occurs every time - you don't have to use FastMM memory manager - basic Delphi memory manager is enough. Here is the code:

Code: Select all

dfm:
object Form1: TForm1
  Left = 0
  Top = 0
  Caption = 'Form1'
  ClientHeight = 449
  ClientWidth = 745
  Color = clBtnFace
  Font.Charset = DEFAULT_CHARSET
  Font.Color = clWindowText
  Font.Height = -11
  Font.Name = 'Tahoma'
  Font.Style = []
  OldCreateOrder = False
  PixelsPerInch = 96
  TextHeight = 13
  object dbg1: TDBGrid
    Left = 0
    Top = 0
    Width = 745
    Height = 408
    Align = alClient
    DataSource = DataSource1
    Options = [dgEditing, dgTitles, dgIndicator, dgColumnResize, dgColLines, dgRowLines, dgTabs, dgConfirmDelete, dgCancelOnExit, dgMultiSelect, dgTitleClick, dgTitleHotTrack]
    TabOrder = 0
    TitleFont.Charset = DEFAULT_CHARSET
    TitleFont.Color = clWindowText
    TitleFont.Height = -11
    TitleFont.Name = 'Tahoma'
    TitleFont.Style = []
  end
  object pnl1: TPanel
    Left = 0
    Top = 408
    Width = 745
    Height = 41
    Align = alBottom
    Caption = 'pnl1'
    TabOrder = 1
    object lblError: TLabel
      Left = 512
      Top = 8
      Width = 3
      Height = 13
      Font.Charset = DEFAULT_CHARSET
      Font.Color = clRed
      Font.Height = -11
      Font.Name = 'Tahoma'
      Font.Style = []
      ParentFont = False
    end
    object Button1: TButton
      Left = 9
      Top = 6
      Width = 75
      Height = 25
      Caption = 'Problem'
      TabOrder = 0
      OnClick = Button1Click
    end
  end
  object OraSession: TOraSession
    Username = 'system'
    Server = 'xe'
    LoginPrompt = False
    Left = 224
    Top = 184
  end
  object OraQuery1: TOraQuery
    Session = OraSession
    SQL.Strings = (
      'select * from u_btk.t_test where test_id = 1')
    CachedUpdates = True
    Options.StrictUpdate = False
    Options.PrepareUpdateSQL = True
    Left = 320
    Top = 200
  end
  object DataSource1: TDataSource
    DataSet = OraQuery1
    OnDataChange = DataSource1DataChange
    Left = 176
    Top = 104
  end
  object OraSQL1: TOraSQL
    Session = OraSession
    SQL.Strings = (
      'delete from u_btk.t_test where test_id = 1')
    Left = 400
    Top = 248
  end
end

pas:
unit Unit1;

interface

uses
  Winapi.Windows, Winapi.Messages, System.SysUtils, System.Variants, System.Classes, Vcl.Graphics,
  Vcl.Controls, Vcl.Forms, Vcl.Dialogs, Data.DB, MemDS, DBAccess, Ora, OraCall, Vcl.Grids, Vcl.DBGrids, Vcl.ExtCtrls,
  Vcl.StdCtrls, DASQLMonitor, OraSQLMonitor, System.Generics.Collections;

type
  PTest = ^TTest;
  TTest = record
    Field: array [0..861] of Byte;
  end;

  TForm1 = class(TForm)
    OraSession: TOraSession;
    OraQuery1: TOraQuery;
    dbg1: TDBGrid;
    DataSource1: TDataSource;
    pnl1: TPanel;
    Button1: TButton;
    OraSQL1: TOraSQL;
    lblError: TLabel;
    procedure Button1Click(Sender: TObject);
    procedure DataSource1DataChange(Sender: TObject; Field: TField);
  private
    FList: TList<PTest>;
  public
    constructor Create(AOwner: TComponent); override;
    destructor Destroy; override;
  published
    procedure MemoryManagerActivitySimulation;
  end;

var
  Form1: TForm1;

implementation

{$R *.dfm}

procedure TForm1.Button1Click(Sender: TObject);
begin
  OraSession.Connect;
  OraQuery1.Open;
  Application.ProcessMessages;
  dbg1.SelectedRows.CurrentRowSelected := True;
  OraSQL1.Execute;
  OraQuery1.RefreshRecord;
  OraSession.Rollback;  // For testing only - we don't want to delete row
  try
    if (dbg1.SelectedRows.Count > 0) and OraQuery1.BookmarkValid(dbg1.SelectedRows.Items[0]) then
      raise Exception.Create('Bookmark valid, for non existing record.');
  finally
    OraSession.Disconnect;
  end;
end;

constructor TForm1.Create(AOwner: TComponent);
begin
  inherited;
  FList := TList<PTest>.Create;
end;

procedure TForm1.DataSource1DataChange(Sender: TObject; Field: TField);
begin
  if OraQuery1.RecordCount = 0 then
    MemoryManagerActivitySimulation;
  if dbg1.SelectedRows.Count > 0 then
    if OraQuery1.BookmarkValid(dbg1.SelectedRows[0]) then
      if OraQuery1.RecordCount = 0 then
        lblError.Caption := 'Bookmark valid for non existing record';
end;

destructor TForm1.Destroy;
var
  i: Integer;
begin
  for i := 0 to FList.Count - 1 do
    FreeMem(FList[i]);
  FList.Free;
  inherited;
end;

procedure TForm1.MemoryManagerActivitySimulation;
var
  i: Integer;
  test: PTest;
  j: Integer;
begin
  for i := 0 to FList.Count - 1 do
    FreeMem(FList[i]);
  FList.Clear;
  FList.Capacity := 100000;
  for i := 0 to FList.Capacity - 1 do
  begin
    GetMem(test, SizeOf(TTest));
    for j := Low(test.Field) to High(test.Field) do
      test.Field[j] := 255;
    FList.Add(test)
  end;
end;

end.

sql:
create table T_TEST
(
  test_id INTEGER
)

Just compile application in Delphi (I'm using DXE4Pro) and click Problem button. Now DataChange event is called only one time - after refreshing deleted record. And in this call dataset is empty - RecordCount property has value 0 but the BookmarkValid function return value True for the bookmark to the deleted record.
At begin of DataChange procedure I made simulation of memory manager activity. Here are some memory block allocated and used to show you, that BookmarkValid function use pointer to freed memory. FastMM make it everytime there is no need to make this manually, but I'm affraid you don't belive in FastMM results.
I hope now you see that the problem is in ODAC code.

regards Adam Siwon

[AlexP]

We have reproduced the problem and will investigate the reasons for such behavior.

[Traptak]

Is this bug fixed in ODAC version 9.2.6?

regards Adam Siwon

[AlexP]

Yes, this problem is already fixed. Please download the latest ODAC version 9.2.6

[Traptak]

Hello Alex,

Some days a go I have upgraded ODAC library (9.3.10) in my environment. After some tests I have found BookmarkValid wrong result error again. I have checked correction you made in TMemData.BookmarkValid function. You have added new condition:

Code: Select all

Bookmark.Item.Order <> -1

This helps for my previous test but not for real situation when in memory are random values. So I wrote new test. In this time while the test running exception: Record not found is raised. Its because BookmarkValid still returns wrong values (True for non existing record). Here is test code:

Code: Select all

  PTest = ^TTest;
  TTest = record
    Field: array [0..861] of Byte;
  end;

  TBookmarkValidTest = class
  private
    FList: TList<PTest>;
    procedure MemoryManagerActivitySimulation;
  public
    constructor Create;
    destructor Destroy; override;
    procedure Execute;
    class procedure TestExecute;
  end;

constructor TBookmarkValidTest.Create;
begin
  FList := TList<PTest>.Create;
end;

destructor TBookmarkValidTest.Destroy;
var
  i: Integer;
begin
  for i := 0 to FList.Count - 1 do
    FreeMem(FList[i]);
  FList.Free;
  inherited;
end;

procedure TBookmarkValidTest.Execute;
var
  vtTest: TVirtualTable;
  i: Integer;
  bookmark: TBookmark;
begin
  vtTest := TVirtualTable.Create(nil);
  try
    vtTest.AddField('Test', ftInteger);
    vtTest.Open;
    for i := 1 to 3 do
    begin
      vtTest.Append;
      vtTest.FieldByName('Test').AsInteger := i;
      vtTest.Post;
    end;

    vtTest.DisableControls;
    bookmark := vtTest.Bookmark;
    while not vtTest.IsEmpty do
      vtTest.Delete;
    MemoryManagerActivitySimulation;
    if vtTest.BookmarkValid(bookmark) then
      vtTest.GotoBookmark(bookmark);
    vtTest.EnableControls;
  finally
    vtTest.Free;
  end;
end;

procedure TBookmarkValidTest.MemoryManagerActivitySimulation;
var
  i: Integer;
  test: PTest;
  j: Integer;
begin
  for i := 0 to FList.Count - 1 do
    FreeMem(FList[i]);
  FList.Clear;
  FList.Capacity := 100000;
  for i := 0 to FList.Capacity - 1 do
  begin
    GetMem(test, SizeOf(TTest));
    for j := Low(test.Field) to High(test.Field) do
      test.Field[j] := 254;
    FList.Add(test)
  end;
end;

class procedure TBookmarkValidTest.TestExecute;
var
  test: TBookmarkValidTest;
begin
  test := Self.Create;
  try
    test.Execute;
  finally
    test.Free;
  end;
end;

To make test just execute TBookmarkValidTest.TestExecute procedure. BookmarkValid function returns true for deleted record. The main problem still exist in the ODAC code. You are trying to use reference to freed object and you try to analyze values from this reference. This is dangerous and may give unpredictable results. I think condition like:
Bookmark.Item.Order <> -1 is not a solution. In real application it can be any value in memory. I think the better solution would be using condition:

Code: Select all

Bookmark.Item.Order > 0

but it is still workaround and my returns wrong values but less than now. In my ODAC source code I have corrected function BookmarkValid by adding condition:

Code: Select all

FRecordCount > 0

If there is no records in DataSet then any bookmark can't be valid.

I hope this helps. Example was compiled and tested using Delphi XE4 pro, ODAC 9.3.10.

best regards
Adam Siwon

[AlexP]

Thank you for the information, we have reproduced the problem. Currently, you can use your workaround. We plan to change the full implementation of Bookmark in one of the next versions.