Ubuntu
cron package

Fix for #151231 leaves tmpfile fd open in actual job

Bug #665912 reported by Jürgen Kreileder
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
cron (Ubuntu)
Fix Released
Undecided
Christian Kastner

Bug Description

Binary package hint: cron

Maverick with cron 3.0pl1-114ubuntu1

I just upgraded a lucid machine to maverick. Now my backup cronjob spits out lots of warnings like:

File descriptor 5 (/tmp/tmpflCpLg8 (deleted)) leaked on lvcreate invocation. Parent PID 7092: /bin/bash
File descriptor 5 (/tmp/tmpflCpLg8 (deleted)) leaked on lvcreate invocation. Parent PID 7092: /bin/bash
File descriptor 5 (/tmp/tmpflCpLg8 (deleted)) leaked on lvcreate invocation. Parent PID 7092: /bin/bash
[...]
File descriptor 5 (/tmp/tmpflCpLg8 (deleted)) leaked on lvremove invocation. Parent PID 7092: /bin/bash
File descriptor 5 (/tmp/tmpflCpLg8 (deleted)) leaked on lvremove invocation. Parent PID 7092: /bin/bash
File descriptor 5 (/tmp/tmpflCpLg8 (deleted)) leaked on lvremove invocation. Parent PID 7092: /bin/bash

Thees are harmless but get sent to stderr => I now get a mail with these warnings each time the job runs. (I can't redirect stderr to /dev/null because I don't want to miss real problems).

It turns out that this problem was introduced to cron with this patch: http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/maverick/cron/maverick/revision/26#do_command.c (included since 3.0pl1-113ubuntu1)
The patch makes cron use a tmpfile to fix bug #151231 and http://bugs.debian.org/155109 (where the patch comes from originally).

The problem is that the file descriptor for this tmpfile is left open in the child which runs the actual cron job. Test case:

$ crontab -l
# stdout to log, stderr to mail
*/1 * * * * /home/jk/test.sh > /tmp/stdout.log
$ cat /home/jk/test.sh
#!/bin/sh
echo normal
echo warning/error >&2
echo
ls -al /proc/$$/fd
echo
# close /tmp/... fd
exec 5>&-
ls -al /proc/$$/fd
echo normal
echo warning/error >&2

After the job has run:
---
$ cat /tmp/stdout.log
normal

total 0
dr-x------ 2 jk jk 0 2010-10-24 16:17 .
dr-xr-xr-x 7 jk jk 0 2010-10-24 16:17 ..
lr-x------ 1 jk jk 64 2010-10-24 16:17 0 -> pipe:[2722425]
l-wx------ 1 jk jk 64 2010-10-24 16:17 1 -> /tmp/stdout.log
lr-x------ 1 jk jk 64 2010-10-24 16:17 10 -> /home/jk/test.sh
lrwx------ 1 jk jk 64 2010-10-24 16:17 2 -> /tmp/tmpflc7JPf (deleted)
lrwx------ 1 jk jk 64 2010-10-24 16:17 5 -> /tmp/tmpflc7JPf (deleted)

total 0
dr-x------ 2 jk jk 0 2010-10-24 16:17 .
dr-xr-xr-x 7 jk jk 0 2010-10-24 16:17 ..
lr-x------ 1 jk jk 64 2010-10-24 16:17 0 -> pipe:[2722425]
l-wx------ 1 jk jk 64 2010-10-24 16:17 1 -> /tmp/stdout.log
lr-x------ 1 jk jk 64 2010-10-24 16:17 10 -> /home/jk/test.sh
lrwx------ 1 jk jk 64 2010-10-24 16:17 2 -> /tmp/tmpflc7JPf (deleted)
normal
---

As you can see, fd 5 points to the tmpfile. It shouldn't be there! (fd 2 is ok)

I'm currently using "exec 5>&-" as a work-around for my backup cronjob - but that's an ugly fix of course.

Related branches

lp:debian/cron
Jürgen Kreileder (jk)
summary: - Fix for #151231 leave fd open in actual job
+ Fix for #151231 leaves tmpfile fd open in actual job
Revision history for this message
Christian Kastner (ckk) wrote :

(I added that patch, which was provided by Justin Pryzby, to cron)

Strange. On my Debian Squeeze system with cron-115 (diff to -114 is only in packaging), I get the following results with the test case you provided:

$ cat /tmp/stdout.log
normal

total 0
dr-x------ 2 chris chris 0 Oct 24 23:43 .
dr-xr-xr-x 7 chris chris 0 Oct 24 23:43 ..
lr-x------ 1 chris chris 64 Oct 24 23:43 0 -> pipe:[43931]
l-wx------ 1 chris chris 64 Oct 24 23:43 1 -> /tmp/stdout
lr-x------ 1 chris chris 64 Oct 24 23:43 10 -> /tmp/test.sh
l-wx------ 1 chris chris 64 Oct 24 23:43 2 -> /tmp/stderr
lrwx------ 1 chris chris 64 Oct 24 23:43 5 -> /tmp/tmpfznXN8E (deleted)

total 0
dr-x------ 2 chris chris 0 Oct 24 23:43 .
dr-xr-xr-x 7 chris chris 0 Oct 24 23:43 ..
lr-x------ 1 chris chris 64 Oct 24 23:43 0 -> pipe:[43931]
l-wx------ 1 chris chris 64 Oct 24 23:43 1 -> /tmp/stdout
lr-x------ 1 chris chris 64 Oct 24 23:43 10 -> /tmp/test.sh
l-wx------ 1 chris chris 64 Oct 24 23:43 2 -> /tmp/stderr
normal
---

Revision history for this message
Jürgen Kreileder (jk) wrote :

The difference is that your example redirects both stdout and stderr while mine only redirected stdout.

Anyway, your test shows the problem too: "5 -> /tmp/tmpfznXN8E" shouldn't be there. There's a close missing somewhere in the child.

Revision history for this message
Christian Kastner (ckk) wrote :

Yes, my bad there.

Anyway, I'm pretty sure I found the issue. However, it appears as though the author of the original patch had attempted this too (without success), and left a warning in there cautioning against this. It appears as if the problem was only an implementation issue, but given the sensitivity of this issue -- the risk of screwing up output mailing, which can be vital -- I'll have to verify this fix in more detail.

If you need this urgently, you can get the current fix here:
http://svn.debian.org/viewsvn/pkg-cron/trunk/do_command.c?r1=550&r2=623

Changed in cron (Ubuntu):
status: New → Confirmed
assignee: nobody → Christian Kastner (ckk)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package cron - 3.0pl1-116ubuntu1

---------------
cron (3.0pl1-116ubuntu1) natty; urgency=low

  * Merge from debian unstable (LP: #696953), remaining changes:
    - debian/control:
      + Requires debhelper >= 7.3.15ubuntu2 (for Upstart).
      + Move MTA,lockfile-progs to Suggests field.
    - debian/cron.upstart: Add Upstart script.
    - debian/{prerm,postinst,postrm}:
      + Don't call update-rc.d,invoke-rc.d and
        /etc/init.d/cron.
    - debian/rules: Call dh_installinit to install Upstart job properly.

cron (3.0pl1-116) unstable; urgency=high

  * Upload with approval from Release Team to get RC bug fixes in Squeeze
    (see http://lists.debian.org/debian-release/2010/12/msg00719.html)
  * do_command.c, popen.c:
    - Use fork() instead of vfork().
  * do_command.c:
    - Close an unused stream in the fork()ed child prior to exec'ing the
      user's command, thereby avoiding an fd leak. Closes: #604181, LP: #665912
      Previously to this, in conjunction with LVM, the fd leak may have the
      effect of the user being spammed by warnings every time a cron job was
      executed.
  * crontab.5:
    - Fixed the example demonstrating how to run a job on a certain weekday of
      the month (date range was off-by-one). Also, the same example contained
      a superfluous escape, resulting in wrong output. Closes: #606325
  * cron.init:
    - Added $named to Should-Start, in case @reboot jobs need DNS resolution.
      Closes: #602903
    - Added nslcd to Should-Start. LP: #27520
 -- Lorenzo De Liso <email address hidden> Mon, 03 Jan 2011 20:32:01 +0100

Changed in cron (Ubuntu):
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.