1629187 - nsExec::Exec return codes are left on the stack

Status: NEW

(Firefox :: Installer, defect, P3)

Description

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

While doing another review I noticed that the code I'd added for uninstalling the update agent (config'd off by default) fails to pop the result from calling nsExec::Exec. From the doc:

Return Value
------------
If nsExec is unable to execute the process, it will return "error"
on the top of the stack, if the process timed out it will return
"timeout", else it will return the return code from the
executed process.

I think I had copied that line from the code that calls the maintenance service uninstaller, it really could just be an ExecWait since the program isn't of the console subsystem so there is no need to suppress the command window.

It looks like most other uses of nsExec::Exec also leave the returned value on the stack. In particular in function un.UninstallServiceIfNotUsed we try to pop off the saved values of $0 and $1, but since the stack may have been unbalanced in between this may not work (which ends up not mattering since the caller doesn't care if $0 and $1 are preserved). In other cases the stack was probably empty at the call site anyway.

It is probably not causing any bugs now, but it might make modifications of the installer/uninstaller hazardous. I haven't done enough analysis to be confident that just adding the Pops won't break something else that's accidentally relying on this, though.