This is a new attempt at fixing vi(1) recovery by actually writing to the recovery file. Previously I restored the SIGALRM method that was deleted in the 90's but wondered if that was still the best way to handle this. Checking and syncing to the recovery every 2 minutes seems arbitrary and overly cautious.
This attempt takes it to the other direction. I'm writing each change to the recovery file immediately after the in-memory database is modified. Though, I can see that this might have a noticeable impact on slower file systems. VIM takes a sort of hybrid approach and writes to the backup every 200 characters or after 4 seconds of idle time. Perhaps this is the best method to not get too far behind while also not hammering the filesystem with quick edits. For now, I'm sticking to the naive approach for review. The diff is smaller than using SIGALRM and more straight forward and I'd like to hear what method might make sense to improve the process. This code would probably be the basis for other improvements. Below is my original explanation of the problem with vi(1)'s recovery. This is a reference to the older SIGALRM diff (I have a version updated to use the atomic signal flags if we want to look at the SIGALRM process instead). https://marc.info/?l=openbsd-tech&m=162940011614049 Tim. ----- 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 exits, 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 ----- Index: common/exf.h =================================================================== RCS file: /cvs/src/usr.bin/vi/common/exf.h,v retrieving revision 1.5 diff -u -p -r1.5 exf.h --- common/exf.h 24 Apr 2015 21:48:31 -0000 1.5 +++ common/exf.h 9 Oct 2021 22:40:17 -0000 @@ -58,7 +58,8 @@ struct _exf { #define F_RCV_NORM 0x020 /* Don't delete recovery files. */ #define F_RCV_ON 0x040 /* Recovery is possible. */ #define F_UNDO 0x080 /* No change since last undo. */ - u_int8_t flags; +#define F_RCV_SYNC 0x100 /* Recovery file sync needed. */ + u_int16_t flags; }; /* Flags to db_get(). */ Index: common/line.c =================================================================== RCS file: /cvs/src/usr.bin/vi/common/line.c,v retrieving revision 1.15 diff -u -p -r1.15 line.c --- common/line.c 6 Jan 2016 22:28:52 -0000 1.15 +++ common/line.c 9 Oct 2021 22:40:17 -0000 @@ -218,7 +218,7 @@ db_delete(SCR *sp, recno_t lno) /* File now modified. */ if (F_ISSET(ep, F_FIRSTMODIFY)) (void)rcv_init(sp); - F_SET(ep, F_MODIFIED); + F_SET(ep, F_MODIFIED | F_RCV_SYNC); /* Update screen. */ return (scr_update(sp, lno, LINE_DELETE, 1)); @@ -266,7 +266,7 @@ db_append(SCR *sp, int update, recno_t l /* File now dirty. */ if (F_ISSET(ep, F_FIRSTMODIFY)) (void)rcv_init(sp); - F_SET(ep, F_MODIFIED); + F_SET(ep, F_MODIFIED | F_RCV_SYNC); /* Log change. */ log_line(sp, lno + 1, LOG_LINE_APPEND); @@ -334,7 +334,7 @@ db_insert(SCR *sp, recno_t lno, char *p, /* File now dirty. */ if (F_ISSET(ep, F_FIRSTMODIFY)) (void)rcv_init(sp); - F_SET(ep, F_MODIFIED); + F_SET(ep, F_MODIFIED | F_RCV_SYNC); /* Log change. */ log_line(sp, lno, LOG_LINE_INSERT); @@ -394,7 +394,7 @@ db_set(SCR *sp, recno_t lno, char *p, si /* File now dirty. */ if (F_ISSET(ep, F_FIRSTMODIFY)) (void)rcv_init(sp); - F_SET(ep, F_MODIFIED); + F_SET(ep, F_MODIFIED | F_RCV_SYNC); /* Log after change. */ log_line(sp, lno, LOG_LINE_RESET_F); 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 9 Oct 2021 22:40:17 -0000 @@ -252,6 +252,8 @@ rcv_sync(SCR *sp, u_int flags) /* Sync the file if it's been modified. */ if (F_ISSET(ep, F_MODIFIED)) { + /* Clear recovery sync flag. */ + F_CLR(ep, F_RCV_SYNC); if (ep->db->sync(ep->db, R_RECNOSYNC)) { F_CLR(ep, F_RCV_ON | F_RCV_NORM); msgq_str(sp, M_SYSERR, Index: ex/ex.c =================================================================== RCS file: /cvs/src/usr.bin/vi/ex/ex.c,v retrieving revision 1.21 diff -u -p -r1.21 ex.c --- ex/ex.c 19 Mar 2016 00:21:28 -0000 1.21 +++ ex/ex.c 9 Oct 2021 22:40:19 -0000 @@ -133,6 +133,10 @@ ex(SCR **spp) msgq(sp, M_ERR, "Interrupted"); } + /* Sync recovery if changes were made. */ + if (F_ISSET(sp->ep, F_RCV_SYNC)) + rcv_sync(sp, 0); + /* * If the last command caused a restart, or switched screens * or into vi, return. 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 9 Oct 2021 22:40:19 -0000 @@ -400,6 +400,10 @@ intr: CLR_INTERRUPT(sp); (void)sp->gp->scr_rename(sp, sp->frp->name, 1); } + /* Sync recovery if changes were made. */ + if (F_ISSET(sp->ep, F_RCV_SYNC)) + rcv_sync(sp, 0); + /* If leaving vi, return to the main editor loop. */ if (F_ISSET(gp, G_SRESTART) || F_ISSET(sp, SC_EX)) { *spp = sp;
