Bug 178836

Created attachment 123623 [details]
short script that attempts to add a crontab entry.

"crontab -e" is really meant for human editors, not scripts.

A much better way of generating crontabs with scripts would be of the form:

  crontab -l | $script | crontab

Use of the 'crontab' command without any arguments replaces the users crontab
with stdin - this is a Red Hat extension to crontab.

So your script could be coded as follows:

---
#!/bin/bash
CRONLINE='50 * * * * echo test'
crontab -l |
( while read line; do
   if [ "$line" = "$CRONLINE" ]; then
      CRONLINE='';
   fi;
   echo $line;
  done
  if [ -n "$CRONLINE" ]; then
     echo "$CRONLINE";
  fi;
) | crontab;
--- 

However, I will look into making crontab access the full 64-bit nanosecond
file modification time available in modern kernels, as an enhancement in
future cron versions.

Show me an strace of a user crontab -e human editor session that completes in
less than 1 second and I'll change this back to a non-enhancement bug :-) .
 

> Show me an strace of a user crontab -e human editor session that 
> completes in less than 1 second and I'll change this back to a 
> non-enhancement bug :-) .

Aha! A challenge!

[august@two august]$ time crontab -e
crontab: no changes made to crontab
 
real    0m0.633s
user    0m0.000s
sys     0m0.000s
[august@two august]$

is my current highscore, (lowscore?) where it actually missed the update.
The command is ddZZ (in vi ; delete a line, save the file and exit. A 
reasonable edit... :) )

I've attached an strace version. 

This made my day.

/August.

Created attachment 123681 [details]
Strace of me editing the crontab in less than 1 sec. :)

I don't agree that stuff like

  crontab -l | $script | crontab

is better. In fact I think it's a bit scary to do that. What if the crontab
processes re-open the crontab file at any point? I'd feel better about

crontab -l > $tempfile ; edit the file ; crontab $tempfile

but then you need a temporary file, with all the hassle that implies. 
(Temp file security, races (several instances of this mechanism doing the same
thing), you name it.)

I like just editing it using -e, it makes things clean and easy.

Oh, and FWIW, I'm down to 490 ms now... :-)

/August.

I had another look. 

First of all, I was a bit off in my judgement of the RHEL4 cron. It copies the
mtime of the existing crontab to the temporary one (that you get to edit), and
checks against that. 

So it's safe against a fast edit. _One_ fast edit. If you try to edit the
crontab twice within a second, it still breaks.
(yes, this is a realworld problem, it's not a totally synthetic test.)

Anyway, attached is a patch that sets the mtime of the temp file to 0 (jan
1,1970), and checks if it's still that after the edit. It works. (Even makes
the code shorter and more obvious)

As an aside, I had a look at what solaris (9) does. Their (non vixie-) cron is
safe against fast edits, but all edits take just over a second. Apparently they
have what's essentially my "!sleep 1" workaround, but it's built in!

Here's a benchmark where you really win. Something for marketing ? :-)

Linux: (RHEL 4, on an old PIII.)

[august@arnold vixie-cron-4.1]$ time for i in `seq 1 100` ; do ./add-entry.sh
2>/dev/null  ; done
                                                                               
                                              
real    0m3.897s
user    0m2.268s
sys     0m1.578s
[august@arnold vixie-cron-4.1]$

Solaris (9, on a v880)

bash-2.05b$ time for i in `seq 1 100` ; do ./add-entry.sh  2>&1  >/dev/null  ; done
 
real    1m45.318s
user    0m0.970s
sys     0m3.080s
bash-2.05b$

/August.

Created attachment 125246 [details]
Patch that zeroes mtime of the temp file before edit, and checks it after.

Hello,
nice time ;-). 
I tried your script and it's 30/70 for install new crontab :) I'll try your 
patch, but I received cron after freeze, so I can push it into FC-7 and then 
maybe to updates.
Thanks for your patience.

updated version is going to be released ... and we are better than Solaris 
now, 100 lines inserted below 10 s on all testing machines ;-)

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on the solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2007-0685.html