Bug in SQLdb...?

[wp_xyz]

Ah, nach deiner Diskussion der FPacketRecords habe ich jetzt auch verstanden, dass, wenn noch keine Datendatei vorhanden war, das folgende Test-Programm im Dataset.Close mit "Access violation reading from address $00000000. - In file 'bufdataset.pas' at line 3445: ScrollResult:=CurrentIndexBuf.ScrollFirst" abstürzt, offenbar weil CurrentIndexBuf nil ist:

Code: Alles auswählen

  Dataset := TBufDataset.Create(nil);
  try
    Dataset.FileName := DB_FILE_NAME;
    if not FileExists(DB_FILE_NAME) then
    begin
      Dataset.FieldDefs.Clear;
      Dataset.FieldDefs.Add('LastName', ftString, 20);
      Dataset.FieldDefs.Add('FirstName', ftString, 20);
      Dataset.FieldDefs.Add('ID', ftString, 4);
      Dataset.CreateDataset;

      Dataset.Open;
      Dataset.AppendRecord(['Einstein', 'Albert', '0003']);
      Dataset.AppendRecord(['Lennon', 'John', '0002']);
      Dataset.AppendRecord(['Monroe', 'Marylin', '0001']);
      Dataset.AppendRecord(['Steinbeck', 'John', '0004']);
      Dataset.AppendRecord(['Joplin', 'Janis', '0005']);
      // <--- hier: Dataset.Close einfügen, um den Crash zu verhindern
    end;

    Dataset.Open;
    ListRecords('Records directly from file');

    Dataset.IndexFieldNames := 'ID';
    ListRecords('Records after setting IndexFieldNames="ID"...');

    Dataset.Refresh;
    ListRecords('... and calling Refresh');
  finally
    Dataset.Close;
    Dataset.Free;
  end; 

Mit einer SQLQuery funktioniert das, aber bei TBufDataset hat man eben diesen Fehler. Das kommt daher, dass die Daten zwar ordnungsgemäß erzeugt werden, aber beim Refresh wieder gelöscht werden, so dass es beim Schreiben in die Datei in Dataset.Close keine Daten mehr gibt. Und offenbar ist dann auch CurrentIndexBuf nil. Das wäre eine weitere Baustelle... Kurzfristig kann man den Absturz aber verhindern, indem nach nach dem Erzeugen des letzten Records ein Dataset.Close einfügt, so dass die Datei (ohne Refresh) geschrieben wird.

Damit bei in-memory-BufDatasets Refresh nicht aufgerufen wird und dabei Daten verloren gehen, würde ich das bereits veränderte TBufDataset.internalRefresh um eine weitere Anweisung ergänzen:

Code: Alles auswählen

procedure TBufDataset.InternalRefresh;
const
  SErrNoInMemoryRefresh = 'In-memory table cannot be refreshed.';
begin
  if (FileName = '') then
    DatabaseError(SErrNoInMemoryRefresh, Self);
  if (ChangeCount>0) then
    CancelUpdates;
  inherited;
end;

[Sieben]

Aber stolpert dann nicht TSQLQuery...? Wie wäre es mit:

Code: Alles auswählen

procedure TBufDataset.InternalRefresh;
const
  SErrNoInMemoryRefresh = 'In-memory table cannot be refreshed.';
begin
  if (DataBase = nil) and (FFileName = '') then
    DatabaseError(SErrNoInMemoryRefresh, Self);
  if (ChangeCount>0) then
    CancelUpdates;
  inherited;
end;

Ich bin mir aber auch nicht sicher, ob das letztlich nicht doch mehr Einschränkungen bedeutet, als wirklich an Absicherung gegen einen Vorgang, den man ohnehin umgehend bereut, gewonnen wird. Jemand könnte noch eine dritte Datenanbindung programmieren wollen und dieses Erbstück verfluchen. Das Wichtigste ist, dass es auch bei diesem unsinnigen Einsatz nicht gleich zu einem Crash kommt.

Gegen den Absturz beim Speichern würde ich vorschlagen:

Code: Alles auswählen

procedure TCustomBufDataset.DoBeforeClose;
begin
  inherited DoBeforeClose;
  if not IsEmpty and (FFileName <> '') then
    SaveToFile(FFileName,dfDefault);
end; 

[wp_xyz]

Dann stolpert aber TSQLQuery:

Code: Alles auswählen

procedure TBufDataset.InternalRefresh;
const
  SErrNoInMemoryRefresh = 'In-memory table cannot be refreshed.';
begin
  if (DataBase = nil) and (FFileName = '') then
    DatabaseError(SErrNoInMemoryRefresh, Self);
  if (ChangeCount>0) then
    CancelUpdates;
  inherited;
end;

Wieso? TSQLQuery stammt von TCustomBufDataset ab und hat sein eigenes InternalRefresh; es bekommt von der in TBufDataset dort gemachten Änderung gar nichts mir.

Ich bin mir aber auch nicht sicher, ob das letztlich nicht doch mehr Einschränkungen bedeutet, als wirklich an Absicherung gegen einen Vorgang, den man ohnehin umgehend bereut, gewonnen wird. Jemand könnte noch eine dritte Datenverbindung programmieren wollen und dieses Erbstück verfluchen. Das Wichtigste ist, dass es auch bei diesem verfehlten Einsatz nicht gleich zu einem Crash kommt.

Aber Datenverlust ist so ziemlich das schlimmste, das man in der Computerei zu befürchten hat. Daher muss es beim Aufruf von Refresh einer In-Memory-Tabelle auf jeden Fall eine Exception mit verständlichem Fehlertext geben, um den User/Entwickler auf den Fehler hinzuweisen. Ich habe zum Beispiel oben am 31.10. 20:00 ein Test-Projekt angehängt, das, wenn man nicht aufpasst, eine leere Datei erzeugt, so dass man bei weiteren Tests mit dieser leeren Datei arbeitet - da wäre eine Fehlermeldung sehr hilfreich gewesen.

Gegen den Absturz beim Speichern würde ich vorschlagen:

Code: Alles auswählen

procedure TCustomBufDataset.DoBeforeClose;
begin
  inherited DoBeforeClose;
  if not IsEmpty and (FFileName <> '') then
    SaveToFile(FFileName,dfDefault);
end; 

Weiß nicht... Es könnte doch auch sein, dass jemand nur eine leere Tabelle erzeugen will, nur mit den Feld- und Index-Definitionen?

[Sieben]

Hast recht, wenn man das erst in TBufDataset und nicht gleich in TCustomBufDataset macht, dann ist es auch nicht im Weg. Und mit dem Speichern auch - war mir nicht bewusst, dass die Datei auch Strukturinfos enthält. Und mit der Absicherung gegen InMemory-Refresh dürfte es da auch keine Abstürze mehr geben. Ich sollte früher in's Bett...

Der Patch hält aber nach wie vor und sieht bei mir jetzt so aus:

Code: Alles auswählen

   [...]
    InitDefaultIndexes;
    InitUserIndexes;
    If FIndexName<>'' then
      FCurrentIndexDef:=TBufDatasetIndex(FIndexes.Find(FIndexName));

    CalcRecordSize;

    FBRecordCount := 0;

    for IndexNr:=0 to FIndexes.Count-1 do
      if Assigned(BufIndexdefs[IndexNr]) then
        With BufIndexes[IndexNr] do
          InitialiseSpareRecord(IntAllocRecordBuffer);

    FAllPacketsFetched := False;

    if (FIndexName='') and (FIndexFieldNames<>'') then
      BuildCustomIndex;

    FOpen:=True;

    // parse filter expression
    ParseFilter(Filter);
    [...] 

FOpen bestimmt den Rückgabewert von IsCurorOpen, das aber erst in ParseFilter erstmalig abgefragt wird. Nicht ganz sicher war ich mir zunächst mit InitialiseSpareRecord(IntAllocRecordBuffer), aber ein Dummy CustomIndex wird bereits in InitDefaultIndexes angelegt, ist also auch schon initialisiert wenn BuildCustomIndex aufgerufen wird.

[wp_xyz]

Ich habe nun alles in einem Bug-Report zusammengefasst: https://gitlab.com/freepascal.org/fpc/s ... sues/40987. Mal sehen, was daraus wird.

[Sieben]

Eigentlich sollte es eine hohe Priorität bekommen, schließlich geht es um einen hässlichen Crash in einer zentralen Datenbankkomponente, andererseits scheint der Einsatz von IndexFieldNames zum Erstellen eines Custom Index nicht wirklich verbreitet und üblich zu sein, sonst wäre doch längst schon mal mehr als einer drüber gestolpert. Auch im Netz findet man zum Thema Sortierung bspw im DBGrid eben bevorzugt die Serverabfrage oder gar den Einsatz der persistenten Indexe, oftmals noch mit Hochsetzen von MaxIndexes. Schau'n mer halt mal...

[wp_xyz]

Michael Van Canneyt hat jetzt den Patch in FPC aufgenommen. Aber da v3.2.4 für weitere Merges geschlossen ist und keine weitere 3.2.x Version mehr geplant ist, kommt der Fix erst in v3.4 ins Release.

[Soner]

Michael Van Canneyt hat jetzt den Patch in FPC aufgenommen. Aber da v3.2.4 für weitere Merges geschlossen ist und keine weitere 3.2.x Version mehr geplant ist, kommt der Fix erst in v3.4 ins Release.

Schade, jetzt werden Leute wieder einige Jahre den gleichen Fehler als Bug melden und man muss viele Jahre Freepascal mit dieser Fehler verwenden.
Der Fehler mit dem TFPHTTPServer den ich als Bug gemeldet habe, wurde vor ca 1,5 Jahren in Entwicklerversion behoben, aber ich bekomme immer noch Benachrichtigungen, weil einige Leute ab und zu unter meiner Bugmeldung schreiben, dass es noch auftaucht, weil sie immer noch die Version 3.2.2 verwenden.
Ich denke man sollte in bestimmten Zeitabständen, z.B. alle halbes Jahr RTL/FCL-Bugfix-Version herausbringen. Neue Versionen mit neue Compilerfunktionen könnte dann herausgebracht werden, wenn es fertig ist.
Man könnte z.B. die erste Versionszahl für Compiler verwenden und die zweite Zahl für RTL/FCL.

[PascalDragon]

Ich denke man sollte in bestimmten Zeitabständen, z.B. alle halbes Jahr RTL/FCL-Bugfix-Version herausbringen. Neue Versionen mit neue Compilerfunktionen könnte dann herausgebracht werden, wenn es fertig ist.

Das große Problem ist, das dies das erste Release seit der Umstellung auf Git und auch einer internen Umstellung unserer Server ist. Da sind in dem Zuge eben einige Schwierigkeiten und Probleme aufgetaucht... Und dann ist Manpower eben auch nicht in großen Mengen vefügbar (so ein Release kommt nicht von selbst zustande).