[Tcsh] tcsh 6.22.00 does not merge history (was: Re: tcsh Deadlock with SIGHUP)

[Christos Zoulas]

Fixed, thanks!

christos

> On Feb 23, 2020, at 3:55 PM, Brett Frankenberger <rbf at rbfnet.com> wrote:
> 
> So I replaced my custom tcsh with the current source and confirmed that
> the fix you implemented prevents the deadlock, but in the process of
> testing, found another bug -- tcsh no longer (as of 6.21) properly
> merges history even when savehist is configured to do so.  (This is
> unrelated to the fix for the SIGHUP deadlock.)
> 
> For example, if you do the following (with savehist = (100 merge lock))
> (although the lock is, I think, not important here):
> 
>  (1) Launch two sessions.
>  (2) In the first session, enter the command "testcommand1".
>  (2) In the second session, enter the command "testcommand2".
>  (3) Exit the first session.
>  (4) Exit the second session.
>  (5) Launch a new session.
> 
> Then, in the new session, the history will only include "testcommand2";
> it will not include "testcommmand1", because when the second sesson
> exited (step 4 above), the history file written by the first session
> (which exited in step 3 above) is not merged.
> 
> This appears to have broken between 6.21.00 and 6.22.00; I believe this
> fix:
>   4. Avoid infinite loops during history loads when merging, print a better
>      error for errors during history load.
> is the cause; specifically, this code in sh.hist.c:
>   - loadhist(fname, 1);
>   + getexit(osetexit);
>   + if (setexit())
>   +     loadhist(fname, 1);
>   + resexit(osetexit);
> 
> It looks to me like "if (setexit())" should be "if (!setexit())".
> (setexit() is ultimately a wrapper around setjmp() or sigsetjmp(), both
> of which return 0 on their first invocation.  So the code as written is
> skipping loadhist() the first time through, which appears to be the
> opposite of what was intended.)
> 
> Replacing "setexit()" with "!setexit()" fixed the merge issue for me.
> Specifically:
> 
> ============
> --- tcsh-6.22.02.orig/sh.hist.c
> +++ tcsh-6.22.02/sh.hist.c
> @@ -1292,7 +1292,7 @@ rechist(Char *fname, int ref)
> #endif
>            }
>            getexit(osetexit);
> -           if (setexit())
> +           if (!setexit())
>                loadhist(fname, 1);
>            resexit(osetexit);
>        }
> ============
> 
>     -- Brett
> 
> On Mon, Jan 20, 2020 at 11:40:49AM -0500, Christos Zoulas wrote:
>> Thanks, it should be fixed now.
>> 
>> christos
>> 
>>> On Jan 20, 2020, at 9:08 AM, Brett Frankenberger <rbf at rbfnet.com> wrote:
>>> 
>>> (Resending what I posted last August ... let me know if there's
>>> anything I can do to get this (or a differnet fix for the same issue)
>>> into the tree.)
>>> 
>>> tcsh can deadlock with itself if savehist is confgured with "merge" and
>>> "lock", and two SIGHUPs are received in rapid succession.  The
>>> mechanism of the deadlock is the first SIGHUP triggers a rechist() and
>>> while that rechist() is executing (and after it has created the lock
>>> file), the second SIGHUP triggers a another rechist() which then waits
>>> forever for the lock the the first rechist() created to be released
>>> (which will never happen).
> 
>    [ . . . ]

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 235 bytes
Desc: Message signed with OpenPGP
URL: <https://mailman.astron.com/pipermail/tcsh/attachments/20200223/dc9df613/attachment.asc>