Skip to content

Conversation

@PhrozenByte
Copy link
Member

My webhoster's shell doesn't provide ps, so I'm receiving mails with sh: ps: command not found errors every 20 minutes. Tracking this down to Talk was great... 😒

This is a quick fix. You should neither use exec(), nor shell_exec(), system(), or passthru(). This is a fix specifically made for my setup which doesn't provide ps. For an actual solution you'll have to silence STDERR for every single command you call - including piped commands. You can't be sure that they don't print to STDERR.

Using pipes makes things way harder, thus you shouldn't rely on grep, awk, sed and others. Rather process the output in PHP. Use proc_open() instead - or one of the many great libraries relying on proc_open(), like symfony/process.

@julien-nc
Copy link
Member

@PhrozenByte Thanks for the suggestion and sorry for the inconvenience.

#4746 partially solves the problem, stderr is silenced, no more awk/grep/pipes.

I think checking the presence of /proc/$PID will work on most systems, I mean better than relying on the presence of the ps command. What do you think?

@PhrozenByte
Copy link
Member Author

In my case this wouldn't make much of a difference since /proc is unavailable on this particular webspace, too. It's just an ordinary webspace, I don't even use Matterbridge (neither there nor on my server).

Anyway. If /proc provides the necessary info I would always recommend to rather use /proc instead of calling shell commands. proc_open() and shell_exec() both create new processes resulting in additional overhead. Okay, we're talking about cronjobs here, so no big deal, but nevertheless. What's more important IMO is the simple fact that it's incredibly hard to get this right. symfony/process is a huge library around proc_open() - because proc_open() isn't trivial. To make bad things worse you can't really rely on anything when calling external commands. They might fail for unexpected reasons which are incredibly hard to debug. Different implementations often support different options (while tracking this down to Talk I saw that you had this issue in the past, right?). Even worse, some implementations have slightly different outputs. It's a total mess. Opening a file descriptor below /proc is way easier and the interface is very stable. So yeah, I'd recommend this instead - if possible of course.

@nickvergessen
Copy link
Member

So this is solved via #4746 or needs a rebase?

@julien-nc
Copy link
Member

@nickvergessen Yes this is solved by #4746.

@julien-nc julien-nc closed this Dec 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review bug feature: integration 📦 Integration with 3rd party (chat) service

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants