Package: tcsh Version: 6.17.02-4.1 Severity: normal Tags: patch
An acknowleged problem with tcsh's savehist merge feature is that it doesn't operate atomically, and so may corrupt the history file if many shells exit at once, such as will happen when logging out of a gnome session with many tcsh terminals open. This became a recurrent problem for me recently. I include a possible patch to deal with this: creating and locking a .history.lock file during savehist merge operations. After applying the patch you can activate the feature by adding "lock" to the savehist value: set savehist=(999 merge lock) As currenly implemented, (999 lock) will be ignored, as will (999 lock merge). It has to be (<number> merge lock). I've used fcntl in order to have some hope that this works for NFS-mounted home directories, but I've no idea whether its use could introduce portability problems --- I've only tried it out on my amd64 Debian Linux system, where it *appears* to work. Hope it proves useful. Regards, Conrad -- System Information: Debian Release: wheezy/sid APT prefers testing APT policy: (500, 'testing') Architecture: amd64 (x86_64) Kernel: Linux 2.6.39-2-amd64 (SMP w/8 CPU cores) Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Versions of packages tcsh depends on: ii libc6 2.13-7 Embedded GNU C Library: Shared lib ii libncurses5 5.9-1 shared libraries for terminal hand tcsh recommends no packages. tcsh suggests no packages. -- no debconf information
diff -rc tcsh-6.17.02~/debian/changelog tcsh-6.17.02/debian/changelog *** tcsh-6.17.02~/debian/changelog 2011-07-06 19:20:17.000000000 +0100 --- tcsh-6.17.02/debian/changelog 2011-07-06 19:20:36.428843121 +0100 *************** *** 1,3 **** --- 1,11 ---- + tcsh (6.17.02-4.1) unstable; urgency=low + + * Non-maintainer upload. + * Add 'lock' option to savehist merge, so it doesn't corrupt the history + when you log out of X with multiple shells open. + + -- Conrad J.C. Hughes (for Debian package stuff) <[email protected]> Wed, 06 Jul 2011 19:20:30 +0100 + tcsh (6.17.02-4) unstable; urgency=high * new maintainer Closes: #592093 diff -rc tcsh-6.17.02~/sh.decls.h tcsh-6.17.02/sh.decls.h *** tcsh-6.17.02~/sh.decls.h 2010-05-12 16:01:52.000000000 +0100 --- tcsh-6.17.02/sh.decls.h 2011-07-06 19:20:39.336895138 +0100 *************** *** 285,290 **** --- 285,291 ---- extern void xclose (int); extern void xclosedir (DIR *); extern int xcreat (const char *, mode_t); + extern int xlockfcntl (int, int, struct flock *); extern struct group *xgetgrgid (gid_t); extern struct passwd *xgetpwnam (const char *); extern struct passwd *xgetpwuid (uid_t); diff -rc tcsh-6.17.02~/sh.hist.c tcsh-6.17.02/sh.hist.c *** tcsh-6.17.02~/sh.hist.c 2010-05-12 17:26:26.000000000 +0100 --- tcsh-6.17.02/sh.hist.c 2011-07-06 19:20:39.336895138 +0100 *************** *** 1214,1219 **** --- 1214,1241 ---- } } + /* Bundle lock file descriptor and path name together for lockfile_cleanup(). + */ + struct lockfile { + int fd; + Char *path; + }; + + /* Close and unlink lock file and free its path. + */ + static void + lockfile_cleanup(void *v_description) + { + struct lockfile *description = v_description; + if (description->fd != -1) { + /* Unlink while we have it open to eliminate race conditions, not that I + * guess it matters. May cause trouble under NT? */ + unlink(short2str(description->path)); + xclose(description->fd); + } + xfree(description->path); + } + /* Save history before exiting the shell. */ void rechist(Char *fname, int ref) *************** *** 1221,1226 **** --- 1243,1249 ---- Char *snum; int fp, ftmp, oldidfds; struct varent *shist; + struct lockfile lock_info; static Char *dumphist[] = {STRhistory, STRmhT, 0, 0}; if (fname == NULL && !ref) *************** *** 1267,1272 **** --- 1290,1310 ---- didfds = 0; if ((shist = adrof(STRsavehist)) != NULL && shist->vec != NULL) if (shist->vec[1] && eq(shist->vec[1], STRmerge)) { + if (shist->vec[2] && eq(shist->vec[2], STRlock)) { + lock_info.path = Strspl(fname, STRdotlock); + lock_info.fd = xcreat(short2str(lock_info.path), 0600); + if (lock_info.fd != -1) { + struct flock lock_details; + lock_details.l_type = F_WRLCK; + lock_details.l_whence = SEEK_SET; + lock_details.l_start = 0; + lock_details.l_len = 0; /* Whole file */ + /* If it fails --- well, we tried... */ + (void) xlockfcntl(lock_info.fd, F_SETLKW, &lock_details); + } + /* Free the path, and if appropriate close&unlink the file. */ + cleanup_push(&lock_info, lockfile_cleanup); + } /* * Unset verbose while we read the history file. From: * [email protected] (Jeffrey Bastian) diff -rc tcsh-6.17.02~/sh.misc.c tcsh-6.17.02/sh.misc.c *** tcsh-6.17.02~/sh.misc.c 2010-05-08 01:41:58.000000000 +0100 --- tcsh-6.17.02/sh.misc.c 2011-07-06 19:20:39.336895138 +0100 *************** *** 553,558 **** --- 553,568 ---- return res; } + int + xlockfcntl(int fd, int cmd, struct flock *lock_details) + { + int res; + + while ((res = fcntl(fd, cmd, lock_details)) == -1 && errno == EINTR) + handle_pending_signals(); + return res; + } + #ifdef HAVE_DUP2 static int xdup2(int fildes, int fildes2) diff -rc tcsh-6.17.02~/tc.const.c tcsh-6.17.02/tc.const.c *** tcsh-6.17.02~/tc.const.c 2010-05-12 16:35:37.000000000 +0100 --- tcsh-6.17.02/tc.const.c 2011-07-06 19:20:39.336895138 +0100 *************** *** 112,117 **** --- 112,119 ---- Char STRmm[] = { '-', 'm', '\0' }; Char STRmr[] = { '-', 'r', '\0' }; Char STRmerge[] = { 'm', 'e', 'r', 'g', 'e', '\0' }; + Char STRlock[] = { 'l', 'o', 'c', 'k', '\0' }; + Char STRdotlock[] = { '.', 'l', 'o', 'c', 'k', '\0' }; Char STRtildothist[] = { '~', '/', '.', 'h', 'i', 's', 't', 'o', 'r', 'y', '\0' };
