Open Bug 917842 Opened 11 years ago Updated 2 years ago

possible false positives in isPidAlive

Categories

(Testing :: Mochitest, defect)

defect

Tracking

(Not tracked)

People

(Reporter: ted, Unassigned)

References

()

Details

We have a function in automation.py called "isPidAlive" that we use to check if child processes are still hanging around after the parent process exits. Reviewing jhammel's refactoring patch in bug 746243 (which moves the definitions around) I realized that this function is ripe for false positives. It simply checks whether a process with that PID is alive, but if the original process exits and a new process is spawned that re-uses that PID it will give a false positive. We should probably be checking a second piece of information, like that the parent PID matches the parent we expect if the process is alive, or that the process filename matches the one we expect.
Depends on: 523208
> It simply checks whether a process with that PID is alive, but... (on non-windows, that is)
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #0) > We have a function in automation.py called "isPidAlive" that we use to check > if child processes are still hanging around after the parent process exits. > Reviewing jhammel's refactoring patch in bug 746243 (which moves the > definitions around) I realized that this function is ripe for false > positives. It simply checks whether a process with that PID is alive, but if > the original process exits and a new process is spawned that re-uses that > PID it will give a false positive. > > We should probably be checking a second piece of information, like that the > parent PID matches the parent we expect if the process is alive, or that the > process filename matches the one we expect. ... or we could output the launch time and see if that is accurate
(In reply to Jeff Hammel [:jhammel] from comment #1) > > It simply checks whether a process with that PID is alive, but... > > (on non-windows, that is) No, it does the same thing on Windows.
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.