mainThe GNU Bourne-Again SHell - Support: sr #108450, jobs.c: off-by-one job id in a...

 
 

sr #108450: jobs.c: off-by-one job id in a warning message

Submitter:  Peter Cordes <pcordes>
Submitted:  Tue 26 Nov 2013 03:30:08 AM UTC
   
 
Category:  None Priority:  5 - Normal
Severity:  3 - Normal Status:  Done
Privacy:  Public Assigned to:  None
Open/Closed:  Open Operating System:  GNU/Linux
* Mandatory Fields

Add a New Comment Rich Markup
   

Tue 26 Nov 2013 03:30:08 AM UTC, original submission:  

GNU bash, version 4.2.25(1)-release (x86_64-pc-linux-gnu)

jobs.c has one warning message where the job ID is missing a +1 before being shown to the user.
jobs.c:892 internal_warning (_("forked pid %d appears in running job %d"), pid, job);

 It should be job + 1, like all the other messages that print a job ID.  (line number from bash-4.3-testing git tree).  Also, the conditions for printing this message should probably be more restricted, see below.

 This message got printed in a case where there wasn't actually a problem.  Might want to re-think having bash keep tracking PIDs for suspended commands that are pipelines, where some parts of the pipeline have already exitted.

 My dad leaves a lot of glimpse | egrep | less pipelines suspended in his shell.  jobs -l shows that bash is still tracking PIDs for all the glimpse and egrep processes, even though only less is still actually running.  glimpse and egrep have long since completed, and aren't around even as zombies to keep a claim on the PID they had.  So eventually that PID will be reused.

 This is what happened in my dad's case.  He ran a command (which happened to be a mkdir) which got the same PID as an egrep from a week ago from one of his suspeneded pipelines.

john@tesla:~$ mkdir ens/ens_membership_files
-bash: warning: forked pid 15312 appears in running job 22

jobs -l showed there was an egrep with a pid of 0, which delete_old_job() set to zero right after printing that warning.

<pre>
...
[22] ... another glimpse | egrep | egrep | less
[23]  15310 Done                    glimpse -FUSB -iw 'legag'
     15311                       | egrep --color=auto -d skip -iwv 'doc|ged|wpd|rtf|htm|html|viminfo'
         0                       | egrep --color=auto -d skip -iv 'database'
     15313 Stopped                 | less -M -i
...
</pre>

bash -c 'echo $$'
confirmed that new processes were getting PIDs just a bit above 15312, so I'm confident PIDs had wrapped around my dad just happened to reuse a PID for a new command in the same shell that bash was still tracking it from a week ago.

 We were both puzzled until a web search on the message found jobs.c, and I figured out what was going on.  It's not a useful warning when bash is only remembering the PID of a process that has already exitted.  I'm not sure it's useful at all for bash to remember PIDs of exitted processes.  Maybe it's useful if you use PIDs as part of tmp filenames.  But it's certainly not worth printing a warning when your shell happens to reuse a PID from an exitted pipeline component.

 I think this warning was intented as more of a sanity check, and probably not something that is telling the user they goofed, like when they try to wait for a stopped job, one of the other warnings from job.c.  That being the case, it might be best to just hide it in an #ifdef DEBUG, if you don't want bash to forget about the PIDs of pipeline components.

 I'm not familiar with what, if anything POSIX has to say about shell job control.  All I know is that that warning message wasn't useful, and was off by one in any case :P.

 Thanks for making such a nice shell, this is probably the first time it's done something weird that wasn't my fault.

Peter Cordes <pcordes>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by chet (Updated the item)
  • -email is unavailable- added by pcordes (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follows 1 latest change.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-11-13 chet StatusNone Done

    Back to the top

    Powered by Savane 3.14-f13d.
    Corresponding source code