Skip to content

Commit

Permalink
#318 problem where multiple notes saved at quit, contention
Browse files Browse the repository at this point in the history
  • Loading branch information
davidbannon committed Oct 16, 2024
1 parent d47209a commit f55854b
Show file tree
Hide file tree
Showing 12 changed files with 142 additions and 67 deletions.
4 changes: 2 additions & 2 deletions source/Tomboy_NG.lpi
Original file line number Diff line number Diff line change
Expand Up @@ -1069,7 +1069,7 @@
</PublishOptions>
<RunParams>
<local>
<CommandLineParams Value="--config-dir=/home/dbannon/.config/tomboy-ng-small"/>
<CommandLineParams Value="--config-dir=/home/dbannon/.config/tomboy-ng-alt"/>
</local>
<environment>
<UserOverrides Count="2">
Expand All @@ -1081,7 +1081,7 @@
<Modes Count="1">
<Mode0 Name="default">
<local>
<CommandLineParams Value="--config-dir=/home/dbannon/.config/tomboy-ng-small"/>
<CommandLineParams Value="--config-dir=/home/dbannon/.config/tomboy-ng-alt"/>
</local>
<environment>
<UserOverrides Count="2">
Expand Down
53 changes: 39 additions & 14 deletions source/editbox.pas
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@
2024/04/14 Extensive rewrite about Links, especially FileLink, remove click on "file://".
2024/06/07 UTF8 bug, where missed UTF8 char ahead of file link.
2024/10/05 Made DeletingThisNote public so SearchForm can delete an Open note.
2024/10/16 Fixed the way that Save on Quit works, no contention !
}


Expand Down Expand Up @@ -621,7 +622,10 @@ TEditBoxForm = class(TForm)
SearchedTerm : string; // If not empty, opening is associated with a search, go straight there.
HaveSeenOnActivate : boolean; // Indicates we have run, eg, CheckForLinks at Activate

// If a new note is a member of Notebook, this holds notebook name until first save.

BusySaving : boolean; // Indicates that the thread that saves the note has not, yet exited.

// If a new note is a member of Notebook, this holds notebook name until first save.
TemplateIs : AnsiString;

// Set True by the delete button so we don't try and save it. Or set from
Expand Down Expand Up @@ -665,14 +669,16 @@ TSaveThread = class(TThread)
and writes it to disk. }
procedure Execute; override;
public
//Title : string; // for debugging purposes, remove ???
TheForm : TEditBoxForm;
TheSL : TStringList;
TheLoc : TNoteUpdateRec; // defined in SaveNote
Constructor Create(CreateSuspended : boolean);
end;

var
EditBoxForm: TEditBoxForm;
BusySaving : boolean; // Indicates that the thread that saves the note has not, yet exited.
// BusySaving : boolean; // Indicates that the thread that saves the note has not, yet exited.

implementation

Expand Down Expand Up @@ -726,18 +732,20 @@ procedure TSaveThread.Execute;
FileStream : TFileStream;
SleepCnt : integer = 0;
begin
// Debugln('TSaveThread.Execute >>> start thread to save : ' + TheLoc.FFName);
while (LockSyncingNow or LockSavingNow) do begin // declared and mostly used in Settings
sleep(100);
inc(SleepCnt);
if SleepCnt > Sleeps then begin
PostMessage(sett.Handle, WM_SYNCMESSAGES, WM_SAVETIMEOUT, 0); // ToDo : if this ever happens, we should mark note as dirty again ! How ?
BusySaving := False;
Debugln('TSaveThread.Execute - ERROR, failed to get Save Lock, Filename : ' + TheLoc.FFName);
TheForm.BusySaving := False;
{$ifdef LINUX} // writeln seems more reliable from a thread than debugln ?
writeln('TSaveThread.Execute --- ERROR ---, failed to get Save Lock, Filename : ' + TheLoc.FFName + ' - ', TheForm.Caption) ;
{$endif}
exit;
end;
end;
LockSavingNow := True;
// debugln('TSaveThread.Execute Started ' + FormatDateTime('hh:nn:ss.zzz', Now()) + ' form=' + TheForm.Caption); // ToDo : remove me
Normaliser := TNoteNormaliser.Create;
Normaliser.NormaliseList(TheSL); // TheSL belongs to the thread.
Normaliser.Free;
Expand All @@ -750,22 +758,29 @@ procedure TSaveThread.Execute;
try
TheSL.SaveToStream(WBufStream);
except on E:Exception do begin
Debugln('TSaveThread.Execute - ERROR, failed to save note : ' + E.Message);
WBufStream.Free;
FileStream.Free;
TheSL.Free;
PostMessage(sett.Handle, WM_SYNCMESSAGES, WM_SAVEERROR, 0); // Will release Lock
exit;
{$ifdef LINUX}
writeln('TSaveThread.Execute ---- ERROR ---, failed to save : ' , TheForm.Caption) ;
writeln(' FileName : '+ TheLoc.FFName + ' - ', E.Message);
{$else}
debugln('TSaveThread.Execute --- ERROR ---, failed to save : ' , TheForm.Caption) ;
debugln(' FileName : '+ TheLoc.FFName + ' - ', E.Message);
{$endif}
WBufStream.Free;
FileStream.Free;
TheSL.Free;
PostMessage(sett.Handle, WM_SYNCMESSAGES, WM_SAVEERROR, 0); // Will release Lock
exit;
end;
end;
finally
WBufStream.Free;
FileStream.Free;
TheSL.Free;
BusySaving := False;
if not Sett.AreClosing then
TheForm.BusySaving := False; // Only necessary if not closing, maybe dangerous if closing.
// debugln('TSaveThread.Execute Finished ' + FormatDateTime('hh:nn:ss.zzz', Now()) + ' form=' + TheForm.Caption); // ToDo : remove me
end;
PostMessage(sett.Handle, WM_SYNCMESSAGES, WM_SAVEFINISHED, 0); // Hmm, no error reporting happening here ?
// Debugln('TSaveThread.Execute >>> End of Thread : ' + TheLoc.FFName);
end;

constructor TSaveThread.Create(CreateSuspended: boolean);
Expand Down Expand Up @@ -2376,24 +2391,30 @@ procedure TEditBoxForm.FormShow(Sender: TObject);
{ called when user manually closes this form. }
procedure TEditBoxForm.FormClose(Sender: TObject; var CloseAction: TCloseAction);
begin
// debugln('TEditBoxForm.FormClose called ' + DateTimeToStr(Now) + ' form=' + Caption); // ToDo : remove me
Release;
end;

procedure TEditBoxForm.FormCloseQuery(Sender: TObject; var CanClose: boolean);
begin
// debugln('TEditBoxForm.FormCloseQuery called ' + DateTimeToStr(Now) + ' form=' + Caption); // ToDo : remove me
CanClose := True;
end;

procedure TEditBoxForm.FormDestroy(Sender: TObject);
begin
// debugln('TEditBoxForm.FormDestroy called ' + FormatDateTime('hh:nn:ss.zzz', Now()) + ' form=' + Caption); // ToDo : remove me
if Undoer <> Nil then Undoer.free;
UnsetPrimarySelection; // tidy up copy on selection.
if (length(NoteFileName) = 0) and (not Dirty) then exit; // A new, unchanged note, no need to save.
if not Kmemo1.ReadOnly then
if not DeletingThisNote then
if (not SingleNoteMode) or Dirty then // We always save, except in SingleNoteMode (where we save only if dirty)
SaveTheNote(Sett.AreClosing); // Jan 2020, just call SaveTheNote, it knows how to record the notebook state
// debugln('TEditBoxForm.FormDestroy marking Nil ' + FormatDateTime('hh:nn:ss.zzz', Now()) + ' form=' + Caption); // ToDo : remove me
SearchForm.NoteClosing(NoteFileName);
Application.ProcessMessages;
// debugln('TEditBoxForm.FormDestroy finished ' + FormatDateTime('hh:nn:ss.zzz', Now()) + ' form=' + Caption); // ToDo : remove me
end;

// Make sure position and size is appropriate.
Expand Down Expand Up @@ -4373,6 +4394,8 @@ function TEditBoxForm.SaveStringList(const SL: TStringList; Loc : TNoteUpdateRec
TheSaveThread := TSaveThread.Create(true);
TheSaveThread.TheLoc := Loc;
TheSaveThread.TheSL := Sl;
//TheSaveThread.Title := Caption; // better to use TheForm (as long as its still exists??)
TheSaveThread.TheForm := self;
TheSaveThread.Start;
// It will clean up after itself.
{$else}
Expand Down Expand Up @@ -4410,15 +4433,17 @@ procedure TEditBoxForm.SaveTheNote(WeAreClosing : boolean = False);
SL : TStringList;
OldFileName : string ='';
Loc : TNoteUpdateRec;
NoteContent : string = ''; // Might hold a lowercase text version of note for searcing purposes.
NoteContent : string = ''; // Might hold a lowercase text version of note for searching purposes.
LineNumb : integer = 0;
FName : string;
ItsANewNote : boolean = false;
//T1, T2, T3, T4, T5, T6, T7 : qword; // Timing shown is for One Large Note.

begin

//TheMainNoteLister.DumpNoteNoteList('TEditBoxForm.SaveTheNote ' + NoteTitle);
if BusySaving then begin
debugln('TEditBoxForm.SaveTheNote declined because BusySaving ' + FormatDateTime('hh:nn:ss.zzz', Now()) + ' form=' + Caption);
MainForm.ShowNotification('Failed to Auto Save', 3000); // inform user via notifications
exit;
end;
Expand Down
1 change: 1 addition & 0 deletions source/kmemo2pdf.lfm
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ object FormKMemo2pdf: TFormKMemo2pdf
Caption = 'Yes, if necessary'
Checked = True
TabOrder = 0
TabStop = True
end
object RadioSimNo: TRadioButton
AnchorSideLeft.Control = GroupBox3
Expand Down
2 changes: 1 addition & 1 deletion source/kmemo2pdf.pas
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ function TFormKMemo2pdf.StartPDF : boolean; // return false and user is show
if IsProp then begin // Prop or variable spaced fonts
if RadioDefault.Checked or (ComboProp.Text = '') then begin // use ones from array
for I := 0 to high(FontsVariable) do
if FontList.Add(FontsVariable[i], false, False, False) then begin // ToDo : gasp ! Fixed = True ?????
if FontList.Add(FontsVariable[i], false, False, False) then begin
TestFontProp := FontsVariable[i];
ComboProp.Text := FontsVariable[i];
break;
Expand Down
1 change: 1 addition & 0 deletions source/mainunit.lfm
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ object MainForm: TMainForm
ClientHeight = 294
ClientWidth = 565
OnClose = FormClose
OnCloseQuery = FormCloseQuery
OnCreate = FormCreate
OnDestroy = FormDestroy
OnKeyDown = FormKeyDown
Expand Down
73 changes: 54 additions & 19 deletions source/mainunit.pas
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
2024/06/01 Dont show menu from TMainForm.TrayIconClick() on Wayland-Gnome as it generats a second
menu when using -plaform xcb (qt5/6). Slightly different that the related KDE issue.
Also, only prevent left click on WAYLAND KDE now, sensible KDEs will work OK
2024/10/16 Fixed the way that Save on Quit works, no contention !
CommandLine Switches
Expand Down Expand Up @@ -174,6 +175,7 @@ TMainForm = class(TForm)
procedure ButtSysTrayHelpClick(Sender: TObject);
procedure CheckBoxDontShowChange(Sender: TObject);
procedure FormClose(Sender: TObject; var {%H-}CloseAction: TCloseAction);
procedure FormCloseQuery(Sender : TObject; var CanClose : Boolean);
procedure FormCreate(Sender: TObject);
procedure FormDestroy(Sender: TObject);
procedure FormKeyDown(Sender: TObject; var Key: Word; Shift: TShiftState);
Expand Down Expand Up @@ -327,38 +329,71 @@ procedure TMainForm.FormCreate(Sender: TObject);
LabelBadNoteAdvice.Caption := '';
end;

procedure TMainForm.FormDestroy(Sender: TObject);
{ -------------- Close process -----------------
First, FormCloseQuery is called, it can but rarely should prevent closure.
Next, FormClose is called.
Finally, when any pending work, perhaps setup by above, FormDestroy is called
}

procedure TMainForm.FormCloseQuery(Sender : TObject; var CanClose : Boolean);
var
AForm : TForm;
NoteIndex : integer;
aPNote : PNote;
begin
freeandnil(CommsServer);
// freeandnil(HelpNotes);
//if HelpList <> Nil then writeln('Help List has ' + inttostr(HelpList.Count));
// freeandnil(HelpList);
Sett.AreClosing:=True;
if assigned(TheMainNoteLister) then begin
AForm := TheMainNoteLister.FindFirstOpenNote(NoteIndex);
while AForm <> Nil do begin
inc(NotesSavedAtClose);
AForm.close;
Application.ProcessMessages; // flush each closure as we go.
aPNote := TheMainNoteLister.GetNote(NoteIndex);
// Wait here until either the form becomes nil or we can read its BusySaving as true;
// Its most likely that progress here is dependent of form becoming Nil ! But not certain ....
while (aPNote^.OpenNote <> Nil) and (not TEditBoxForm(aPNote^.OpenNote).BusySaving) do
Sleep(2);

(* while (TheMainNoteLister.NoteList.Items[NoteIndex]^.OpenNote <> nil) // just ugly ....
and (not TEditBoxForm(TheMainNoteLister.NoteList.Items[NoteIndex]^.OpenNote).BusySaving)
do sleep(2); *)

sleep(2); // Long enough for EditBox to launch a save thread ??
AForm := TheMainNoteLister.FindNextOpenNote(NoteIndex);
end;
end;
end;

procedure TMainForm.FormClose(Sender: TObject; var CloseAction: TCloseAction);
var
{$ifdef LCLGTK2}
c: PGtkClipboard;
t: string;
{$endif}
// OutFile : TextFile;
AForm : TForm;
{$ifdef LCLGTK2}
c: PGtkClipboard;
t: string;
{$endif}
QuitDelay : integer = 0;
begin
//debugln('TMainForm.FormClose - at user request'); // ToDo : remove this
Application.ProcessMessages; // seems necessary to ensure we count all closed notes.
{$ifdef LCLGTK2}
c := gtk_clipboard_get(GDK_SELECTION_CLIPBOARD);
t := Clipboard.AsText;
gtk_clipboard_set_text(c, PChar(t), Length(t));
gtk_clipboard_store(c);
{$endif}
Sett.AreClosing:=True;
if assigned(TheMainNoteLister) then begin
AForm := TheMainNoteLister.FindFirstOpenNote();
while AForm <> Nil do begin
AForm.close;
AForm := TheMainNoteLister.FindNextOpenNote();
end;
while NotesSavedAtClose > 0 do begin // Wait while any pending note saves happen
sleep(20); // In practise, should never happen ??
inc(QuitDelay);
if QuitDelay > 500 then begin
showmessage('ERROR, delayed note saving, Unsaved='+ inttostr(NotesSavedAtClose));
break;
end;
end;
if QuitDelay > 0 then
debugln('TMainForm.FormClose Warning saving delay ' + inttostr(QuitDelay*20) + 'mS');
end;

procedure TMainForm.FormDestroy(Sender: TObject);
begin
freeandnil(CommsServer);
end;

procedure TMainForm.CommMessageReceived(Sender : TObject);
Expand Down
22 changes: 14 additions & 8 deletions source/note_lister.pas
Original file line number Diff line number Diff line change
Expand Up @@ -389,9 +389,9 @@ TNoteLister = class
not find a) the Entry in NotebookList or b) the entry had a blank template. }
function NotebookTemplateID(const NotebookName : ANSIString) : AnsiString;
{ Returns the Form of first open note and sets internal pointer to it, Nil if none found }
function FindFirstOpenNote(): TForm;
function FindFirstOpenNote(out OpenIndex : integer) : TForm;
{ Call after FindFirstOpenNote(), it will return the next one or Nil if no more found }
function FindNextOpenNote() : TForm;
function FindNextOpenNote(out NoteIndex : integer) : TForm;
{ Returns the ID of first note that should be opened on startup internal pointer
(which is same interger as FindFirstOpenNate) to it, '' if none found }
function FindFirstOOSNote(out NTitle, NID: ANSIstring): boolean;
Expand Down Expand Up @@ -1462,27 +1462,33 @@ function TNoteLister.IsIDPresent(ID: string): boolean;
exit(True);
end;

function TNoteLister.FindFirstOpenNote(): TForm;
function TNoteLister.FindFirstOpenNote(out OpenIndex : integer): TForm;
begin
OpenNoteIndex:=0;
while OpenNoteIndex < NoteList.Count do
if NoteList.Items[OpenNoteIndex]^.OpenNote <> nil then
if NoteList.Items[OpenNoteIndex]^.OpenNote <> nil then begin
OpenIndex := OpenNoteIndex;
exit(NoteList.Items[OpenNoteIndex]^.OpenNote)
else inc(OpenNoteIndex);
end else inc(OpenNoteIndex);
result := nil;
OpenNoteIndex := -1;
OpenIndex := OpenNoteIndex;
end;

function TNoteLister.FindNextOpenNote(): TForm;
function TNoteLister.FindNextOpenNote(out NoteIndex : integer): TForm;
begin
if OpenNoteIndex < 0 then exit(Nil);
inc(OpenNoteIndex);
while OpenNoteIndex < NoteList.Count do
if NoteList.Items[OpenNoteIndex]^.OpenNote <> nil then
while OpenNoteIndex < NoteList.Count do begin
if NoteList.Items[OpenNoteIndex]^.OpenNote <> nil then begin
NoteIndex := OpenNoteIndex;
exit(NoteList.Items[OpenNoteIndex]^.OpenNote)
end
else inc(OpenNoteIndex);
end;
result := nil;
OpenNoteIndex := -1;
NoteIndex := OpenNoteIndex;
end;

function TNoteLister.FindFirstOOSNote(out NTitle, NID : ANSIstring): boolean;
Expand Down
Loading

0 comments on commit f55854b

Please sign in to comment.