Re: BUG: Zsh loses history entries since 2015

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Thu, Apr 3, 2025 at 2:32 PM Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
>
> Typically in cases of a connection shutdown the signal would be
> SIGHUP, potentially followed by a SIGKILL if it's a local terminal in
> the window/session manager rather than a remote one via SSH.

Looking closer at this, unless we're queuing signals the immediate
response on a SIGHUP is to exit.

More on this below.

> On closer look I am reminded that readhistfile() is called from within
> savehistfile(), so this should be clarified as specifically "happens
> during write-back".

I think there's an opportunity to do a little more error-handling
specifically when readhistfile() has failed during savehistfile().
See patch.

> However, I can't think of anything useful to do there ... with
> hist-save-by-copy it should (already) just be leaving the ".new" file
> behind and (because already dead) skipping the rename over the old
> file.

When savehistfile() is doing straightforward write-back, it only stops
if fprintf() or fputc() returns -1.  It feels like there's a potential
race condition there, but it'd require some queue_signals fiddling or
similar to resolve (or at least reduce) it.  Look for the
extended_history condition + fprintf() to see where I mean.  I haven't
attempted any changes with that, but a SIGHUP here certainly appears
as if it would leave a truncated history file if HIST_SAVE_BY_COPY is
not set, and a SIGINT probably so.

I further suspect there's a logic error at the end of the
readhistline() loop in readhistfile(), proposed fix in the patch.  I
don't believe that would change about the cases that were already
being caught.

Another thing I haven't addressed (and am not sure how to) -- if the
history operation is stopped (TSTP or STOP signal) then when resumed
its file positions may no longer have any relation to the actual
files.  I suppose in theory the fact that it would lockhistfile()
before being stopped should mean that no other changes can apply
before it gets restarted?  SIGSTOP cannot be ignored or trapped, but
it might be possible to set system calls to not auto-resume, so we'd
be able to detect the event.

diff --git a/Src/hist.c b/Src/hist.c
index fa1ede3f0..00bdbb2b8 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -2838,11 +2838,12 @@ readhistfile(char *fn, int err, int readflags)
 	     */
 	    if (uselex || remeta)
 		freeheap();
-	    if (errflag & ERRFLAG_INT) {
-		/* Can't assume fast read next time if interrupted. */
-		lasthist.interrupted = 1;
+	    if (errflag & ERRFLAG_INT)
 		break;
-	    }
+	}
+	if (errflag & ERRFLAG_INT) {
+	    /* Can't assume fast read next time if interrupted. */
+	    lasthist.interrupted = 1;
 	}
 	if (start && readflags & HFILE_USE_OPTIONS) {
 	    zsfree(lasthist.text);
@@ -3108,7 +3109,9 @@ savehistfile(char *fn, int err, int writeflags)
 		hist_ignore_all_dups |= isset(HISTSAVENODUPS);
 		readhistfile(fn, err, 0);
 		hist_ignore_all_dups = isset(HISTIGNOREALLDUPS);
-		if (histlinect)
+		if (errflag & ERRFLAG_INT)
+		    ret = -1;
+		else if (histlinect)
 		    savehistfile(fn, err, 0);
 
 		pophiststack();