trondd <tro...@kagu-tsuchi.com> wrote:
> trondd <tro...@kagu-tsuchi.com> wrote:
>
> >
> > While investigating an occasional crash when recovering a file with 'vi -r'
> > after a power failure, I noticed that the recovery files are actually never
> > updated during an editing session. The recovery files are created upon
> > initial modification of the file which saves the state of the file at the
> > time of the edit. You can work on a file for as long as you want and even
> > write it to disk but the recovery file is never updated. If the session is
> > then lost due to power failure or a SIGKILL and you attempt to recover with
> > -r, you'll be presented with the contents of the file from that first edit.
> > It won't contain unsaved changes nor even any changes manually written to
> > disk to the original file. Accepting the recovered version would lose all
> > of your work.
> >
> > Reading the vi docs, man page, and source comments in the OpenBSD tree, they
> > all mention the use of SIGALRM to periodically save changes to the recovery
> > file. However, the code never sets up a handler or captures SIGALRM. It
> > only ever updates the recovery file on a SIGTERM but then also exists, I
> > guess to cover the case of an inadvertent clean system shutdown.
> >
> > I dug through an nvi source repository[0] that seemed to cover it's entire
> > history and it seems this functionality was lost somewhere around 1994 and I
> > don't see it having been replaced by anything else. Our version seems to be
> > from 1996 and editors/nvi in ports still lacks code to update the recovery
> > file, as well.
> >
> > What I've done is re-implement periodic updates to the recovery file using
> > SIGALRM and a timer like the original implementation but rewritten a bit to
> > fit the newer source file layout and event handling. That keeps the
> > recovery
> > updated every 2 minutes. Then it seemed silly to be able to write changes
> > to
> > the original file and if a crash happens before the next SIGALRM, recovery
> > would try to roll you back to before those saved changes. So I've also
> > added
> > a call to sync the recovery file if you explicitly write changes to disk. I
> > don't think the recovery system should try to punish you for actively saving
> > your work even if it is only at most 2 minutes worth.
> >
> > Comments or feedback? I'm unsure I've covered all caveats with this code or
> > if there are vi/ex usecases where it won't work correctly. For testing,
> > I've
> > covered my usage and several scenarios I could contrive but I don't
> > regularly
> > use ex, for example, or change many options from the default. I've been
> > running with this code for a week. And I suppose there must be a reason no
> > one has noticed or cared about this for over 20 years. Everyone else uses
> > vim, I guess?
> >
> > Tim.
> >
> > [0] https://repo.or.cz/nvi.git
> >
>
> Got positive testing from one user. Anyone else interested? Or any other
> feedback?
>
> Tim.
>
Throwing this hat back into the ring post-release. Still only gotten user
feedback so far.
Tim.
Index: cl/cl.h
===================================================================
RCS file: /cvs/src/usr.bin/vi/cl/cl.h,v
retrieving revision 1.10
diff -u -p -r1.10 cl.h
--- cl/cl.h 27 May 2016 09:18:11 -0000 1.10
+++ cl/cl.h 26 Dec 2020 17:47:32 -0000
@@ -30,8 +30,9 @@ typedef struct _cl_private {
#define INDX_HUP 0
#define INDX_INT 1
#define INDX_TERM 2
-#define INDX_WINCH 3
-#define INDX_MAX 4 /* Original signal information. */
+#define INDX_ALRM 3
+#define INDX_WINCH 4
+#define INDX_MAX 5 /* Original signal information. */
struct sigaction oact[INDX_MAX];
enum { /* Tty group write mode. */
@@ -48,8 +49,9 @@ typedef struct _cl_private {
#define CL_SIGHUP 0x0020 /* SIGHUP arrived. */
#define CL_SIGINT 0x0040 /* SIGINT arrived. */
#define CL_SIGTERM 0x0080 /* SIGTERM arrived. */
-#define CL_SIGWINCH 0x0100 /* SIGWINCH arrived. */
-#define CL_STDIN_TTY 0x0200 /* Talking to a terminal. */
+#define CL_SIGALRM 0x0100 /* SIGALRM arrived. */
+#define CL_SIGWINCH 0x0200 /* SIGWINCH arrived. */
+#define CL_STDIN_TTY 0x0400 /* Talking to a terminal. */
u_int32_t flags;
} CL_PRIVATE;
Index: cl/cl_main.c
===================================================================
RCS file: /cvs/src/usr.bin/vi/cl/cl_main.c,v
retrieving revision 1.33
diff -u -p -r1.33 cl_main.c
--- cl/cl_main.c 5 May 2016 20:36:41 -0000 1.33
+++ cl/cl_main.c 26 Dec 2020 17:47:32 -0000
@@ -239,6 +239,14 @@ h_term(int signo)
}
static void
+h_alrm(int signo)
+{
+ GLOBAL_CLP;
+
+ F_SET(clp, CL_SIGALRM);
+}
+
+static void
h_winch(int signo)
{
GLOBAL_CLP;
@@ -264,6 +272,7 @@ sig_init(GS *gp, SCR *sp)
if (setsig(SIGHUP, &clp->oact[INDX_HUP], h_hup) ||
setsig(SIGINT, &clp->oact[INDX_INT], h_int) ||
setsig(SIGTERM, &clp->oact[INDX_TERM], h_term) ||
+ setsig(SIGALRM, &clp->oact[INDX_ALRM], h_alrm) ||
setsig(SIGWINCH, &clp->oact[INDX_WINCH], h_winch)
)
err(1, NULL);
@@ -271,6 +280,7 @@ sig_init(GS *gp, SCR *sp)
if (setsig(SIGHUP, NULL, h_hup) ||
setsig(SIGINT, NULL, h_int) ||
setsig(SIGTERM, NULL, h_term) ||
+ setsig(SIGALRM, NULL, h_alrm) ||
setsig(SIGWINCH, NULL, h_winch)
) {
msgq(sp, M_SYSERR, "signal-reset");
@@ -313,6 +323,7 @@ sig_end(GS *gp)
(void)sigaction(SIGHUP, NULL, &clp->oact[INDX_HUP]);
(void)sigaction(SIGINT, NULL, &clp->oact[INDX_INT]);
(void)sigaction(SIGTERM, NULL, &clp->oact[INDX_TERM]);
+ (void)sigaction(SIGALRM, NULL, &clp->oact[INDX_ALRM]);
(void)sigaction(SIGWINCH, NULL, &clp->oact[INDX_WINCH]);
}
Index: cl/cl_read.c
===================================================================
RCS file: /cvs/src/usr.bin/vi/cl/cl_read.c,v
retrieving revision 1.21
diff -u -p -r1.21 cl_read.c
--- cl/cl_read.c 27 May 2016 09:18:11 -0000 1.21
+++ cl/cl_read.c 26 Dec 2020 17:47:32 -0000
@@ -62,7 +62,7 @@ retest: if (LF_ISSET(EC_INTERRUPT) || F_
evp->e_event = E_TIMEOUT;
return (0);
}
- if (F_ISSET(clp, CL_SIGHUP | CL_SIGTERM | CL_SIGWINCH)) {
+ if (F_ISSET(clp, CL_SIGHUP | CL_SIGTERM | CL_SIGWINCH | CL_SIGALRM)) {
if (F_ISSET(clp, CL_SIGHUP)) {
evp->e_event = E_SIGHUP;
return (0);
@@ -81,6 +81,11 @@ retest: if (LF_ISSET(EC_INTERRUPT) || F_
return (0);
}
/* No real change, ignore the signal. */
+ }
+ if (F_ISSET(clp, CL_SIGALRM)) {
+ F_CLR(clp, CL_SIGALRM);
+ evp->e_event = E_SIGALRM;
+ return (0);
}
}
Index: common/key.c
===================================================================
RCS file: /cvs/src/usr.bin/vi/common/key.c,v
retrieving revision 1.19
diff -u -p -r1.19 key.c
--- common/key.c 18 Apr 2017 01:45:35 -0000 1.19
+++ common/key.c 26 Dec 2020 17:47:32 -0000
@@ -542,6 +542,10 @@ loop: if (gp->scr_event(sp, argp,
v_sync(sp, RCV_ENDSESSION | RCV_PRESERVE |
(argp->e_event == E_SIGTERM ? 0: RCV_EMAIL));
return (1);
+ case E_SIGALRM:
+ /* Sync to recovery if the file has been modified. */
+ v_sync(sp, 0);
+ return(0);
case E_TIMEOUT:
istimeout = 1;
break;
@@ -756,6 +760,9 @@ v_event_err(SCR *sp, EVENT *evp)
break;
case E_WRITE:
msgq(sp, M_ERR, "Unexpected write event");
+ break;
+ case E_SIGALRM:
+ msgq(sp, M_ERR, "Unexpected SIGALRM event");
break;
/*
Index: common/key.h
===================================================================
RCS file: /cvs/src/usr.bin/vi/common/key.h,v
retrieving revision 1.8
diff -u -p -r1.8 key.h
--- common/key.h 27 May 2016 09:18:11 -0000 1.8
+++ common/key.h 26 Dec 2020 17:47:32 -0000
@@ -42,6 +42,7 @@ typedef enum {
E_QUIT, /* Quit. */
E_REPAINT, /* Repaint: e_flno, e_tlno set. */
E_SIGHUP, /* SIGHUP. */
+ E_SIGALRM, /* SIGALRM. */
E_SIGTERM, /* SIGTERM. */
E_STRING, /* Input string: e_csp, e_len set. */
E_TIMEOUT, /* Timeout. */
Index: common/recover.c
===================================================================
RCS file: /cvs/src/usr.bin/vi/common/recover.c,v
retrieving revision 1.30
diff -u -p -r1.30 recover.c
--- common/recover.c 22 Jul 2019 12:39:02 -0000 1.30
+++ common/recover.c 26 Dec 2020 17:47:32 -0000
@@ -30,6 +30,7 @@
#include <limits.h>
#include <paths.h>
#include <pwd.h>
+#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -180,6 +181,7 @@ int
rcv_init(SCR *sp)
{
EXF *ep;
+ struct itimerval value;
recno_t lno;
ep = sp->ep;
@@ -219,6 +221,18 @@ rcv_init(SCR *sp)
/* Turn off the owner execute bit. */
(void)chmod(ep->rcv_path, S_IRUSR | S_IWUSR);
+ if (!F_ISSET(sp->gp, G_RECOVER_SET)) {
+ /* Start the recovery timer. */
+ value.it_interval.tv_sec = value.it_value.tv_sec = RCV_PERIOD;
+ value.it_interval.tv_usec = value.it_value.tv_usec = 0;
+ if (setitimer(ITIMER_REAL, &value, NULL)) {
+ msgq(sp, M_ERR,
+ "Error: setitimer: %s", strerror(errno));
+ goto err;
+ }
+ }
+
+
/* We believe the file is recoverable. */
F_SET(ep, F_RCV_ON);
return (0);
@@ -244,6 +258,7 @@ rcv_sync(SCR *sp, u_int flags)
EXF *ep;
int fd, rval;
char *dp, buf[1024];
+ sigset_t mask, omask;
/* Make sure that there's something to recover/sync. */
ep = sp->ep;
@@ -252,12 +267,21 @@ rcv_sync(SCR *sp, u_int flags)
/* Sync the file if it's been modified. */
if (F_ISSET(ep, F_MODIFIED)) {
+ /*
+ * Prevent SIGALRM during sync (which I am not sure
+ * would actually matter since it just creates an event).
+ */
+ sigemptyset(&mask);
+ sigaddset(&mask, SIGALRM);
+ sigprocmask(SIG_BLOCK, &mask, &omask);
if (ep->db->sync(ep->db, R_RECNOSYNC)) {
F_CLR(ep, F_RCV_ON | F_RCV_NORM);
msgq_str(sp, M_SYSERR,
ep->rcv_path, "File backup failed: %s");
+ sigprocmask(SIG_SETMASK, &omask, NULL);
return (1);
}
+ sigprocmask(SIG_SETMASK, &omask, NULL);
/* REQUEST: don't remove backing file on exit. */
if (LF_ISSET(RCV_PRESERVE))
Index: ex/ex_txt.c
===================================================================
RCS file: /cvs/src/usr.bin/vi/ex/ex_txt.c,v
retrieving revision 1.17
diff -u -p -r1.17 ex_txt.c
--- ex/ex_txt.c 30 Apr 2020 10:40:21 -0000 1.17
+++ ex/ex_txt.c 26 Dec 2020 17:47:32 -0000
@@ -117,6 +117,7 @@ newtp: if ((tp = text_init(sp, NULL, 0,
break;
case E_ERR:
goto err;
+ case E_SIGALRM:
case E_REPAINT:
case E_WRESIZE:
continue;
Index: ex/ex_write.c
===================================================================
RCS file: /cvs/src/usr.bin/vi/ex/ex_write.c,v
retrieving revision 1.13
diff -u -p -r1.13 ex_write.c
--- ex/ex_write.c 6 Jan 2016 22:28:52 -0000 1.13
+++ ex/ex_write.c 26 Dec 2020 17:47:32 -0000
@@ -127,6 +127,9 @@ exwr(SCR *sp, EXCMD *cmdp, enum which cm
int flags;
char *name, *p = NULL;
+ /* Sync recovery file, too. */
+ rcv_sync(sp, 0);
+
NEEDFILE(sp, cmdp);
/* All write commands can have an associated '!'. */
Index: vi/v_replace.c
===================================================================
RCS file: /cvs/src/usr.bin/vi/vi/v_replace.c,v
retrieving revision 1.9
diff -u -p -r1.9 v_replace.c
--- vi/v_replace.c 6 Jan 2016 22:28:52 -0000 1.9
+++ vi/v_replace.c 26 Dec 2020 17:47:32 -0000
@@ -125,6 +125,9 @@ next: if (v_event_get(sp, &ev, 0, 0))
case E_INTERRUPT:
/* <interrupt> means they changed their minds. */
return (0);
+ case E_SIGALRM:
+ /* Do nothing. */
+ goto next;
case E_WRESIZE:
/* <resize> interrupts the input mode. */
v_emsg(sp, NULL, VIM_WRESIZE);
Index: vi/v_txt.c
===================================================================
RCS file: /cvs/src/usr.bin/vi/vi/v_txt.c,v
retrieving revision 1.34
diff -u -p -r1.34 v_txt.c
--- vi/v_txt.c 30 Apr 2020 10:40:21 -0000 1.34
+++ vi/v_txt.c 26 Dec 2020 17:47:32 -0000
@@ -502,6 +502,9 @@ next: if (v_event_get(sp, evp, 0, ec_fla
case E_WRESIZE:
/* <resize> interrupts the input mode. */
v_emsg(sp, NULL, VIM_WRESIZE);
+ case E_SIGALRM:
+ /* Don't repeat last input. */
+ goto next;
/* FALLTHROUGH */
default:
if (evp->e_event != E_INTERRUPT && evp->e_event != E_WRESIZE)
Index: vi/vi.c
===================================================================
RCS file: /cvs/src/usr.bin/vi/vi/vi.c,v
retrieving revision 1.22
diff -u -p -r1.22 vi.c
--- vi/vi.c 24 Jan 2019 15:09:41 -0000 1.22
+++ vi/vi.c 26 Dec 2020 17:47:32 -0000
@@ -1197,6 +1197,9 @@ v_key(SCR *sp, int command_events, EVENT
break;
case E_WRESIZE:
return (GC_ERR);
+ case E_SIGALRM:
+ /* Prevent unexpected sigalarm message. */
+ return (GC_ERR);
case E_QUIT:
case E_WRITE:
if (command_events)
