[pulseaudio-tickets] [PulseAudio] #158: possibly dangerous use of exit() in a shared library?

[PulseAudio]

#158: possibly dangerous use of exit() in a shared library?
-----------------------+----------------------------------------------------
  Reporter:  pmasiar   |       Owner:  lennart
      Type:  defect    |      Status:  closed 
  Priority:  low       |   Milestone:         
 Component:  libpulse  |    Severity:  minor  
Resolution:  invalid   |    Keywords:         
-----------------------+----------------------------------------------------
Changes (by lennart):

  * status:  new => closed
  * resolution:  => invalid

Comment:

 Uh. The guy who posted that obviously has not much clue about writing
 software or about memory management.

 There are three kinds of software. That which handles OOM gracefully and
 is carefully debugged to make sure it continues to run -- but with
 graceful degradation if no memory is available anymore. Then, there is
 software that aborts on OOM. That's almost any free desktop software,
 especially the one written using libraries like GLib/Gnome which uses
 aborting malloc() almost exclusively (and Qt does it much the same way).
 And finally there's the software which uses plain malloc() but ignores
 errors and segfaults on OOM. Unfortunately the latter style of programming
 is one of the most popular. Now, clueful programmers know about these
 issues and carefully choose in which of the first two styles to program.
 For system programs like init, dbus and suchlike clueful programmers
 usually choose the first style, for desktop programs, and other not
 crucial components they choose aborting malloc(). Why? First, because it
 is much easier to program with. And second, because it doesn't hide or
 abritrarily escalate errors.

 Also note that due to overcommiting on Linux OOM errors are generally not
 handled by malloc() returning NULL, but by killing processes via the OOM
 killer. So spending much time thinking about handling malloc() returning
 NULL is generally a waste of it. It just doesn't happen, unless you
 disable overcommit, or you enforce resource limits. Disabling overcommit
 is only useful on some very specific (usually embedded) applications. It's
 mostly paranoia. So in effect, aborting if malloc() returns NULL just
 makes all situations of OOM behave identically.

 Programs should be optimized to not lose any data if they crash. Every
 non-trivial program has bugs, and thus the priority should be to make them
 not lose any data in the worst of the cases. And thus, in many cases a
 clean abort on OOM is the better choice then continuing with half
 completed operations -- just for the sake of the consistency of the data
 stored on disk.

 So, to be frank: I think that most of the software I wrote makes the right
 choices with handling OOM. In Avahi, the daemon uses aborting malloc(),
 because network service discovery is clearly not a crucial part of the
 system. So if we get into OOM we should be killed -- as one of the first
 processes -- to free up memory for more imporant processes.  However, the
 Avahi client libraries are carefully programmed to deal with malloc()
 returning NULL. Why? Because they are used in quite a bit of system-level
 software where an aborting malloc would not be a good thing. For the PA
 case,  I chose to always use aborting malloc. Why? Because audio *never*
 is a crucial component of the system. If audio crashes due to what reason
 whatsoever all you lose is that your music stops to play -- but no data is
 lost. And thus, audio software should be the first one to be killed, too.

 A lot of unix software defines a wrapper to malloc() which is called
 xmalloc(), which implements the aborting malloc. It's one of the moston
 common idioms in C programs on Unix.

 Oh, and the the guy is asking why so many systems define their own
 malloc() wrapper? Precisely to implement an aborting malloc(). And also
 for portability reasons, i.e. some systems abort on malloc(0), others
 return NULL, others return a single constant but valid pointer and even
 others actually allocate a zero-length memory block.

 Also note, that PA doesn't call exit(). It calls _exit() which is a small
 difference.

 Quoting from his post:

 "crummy, amateurish programming". I like that. I wonder what kind of
 really important, really safe software that "j_g" guy ever wrote. I
 certainly haven't heard of it.

 I hope he doesn't really use Ubuntu, because most of the software running
 on Ubuntu desktops uses an aborting malloc(), by using GLib. And thus must
 be "crummy, amateurish".

   "And anyone who makes such a shared library a part of his operating
 system should be shot dead, and then stabbed through the heart with a
 stake just to make sure he's really dead."

 Yes, that's constructive. Tell j_g I love him too. Even though he's a
 clueless FUD-spreading fool.

   "God help us if this pulse audio thing is ever chosen to replace ALSA in
 the kernel. This will make Linux stability and reliability go to hell if
 you have operating system calls terminating apps at will."

 Hehe. it's getting better. PA does not live in kernel space and doesn't
 replace ALSA in any way. It sits on top of ALSA. I am not sure what kind
 of illusions that guy has about the Linux kernel and the general code
 quality of Linux.

 Oh, you wanna know something? Even ALSA doesn't properly check the return
 values of all malloc()s. Oh! My! God! Hell is freezing over! Heaven's
 sake!

 I am sorry, I am no going to sign up to some random ubuntu forum, just to
 reply to some clueless fool. However, feel free to link or copy my reply
 into that forum.

-- 
Ticket URL: <http://pulseaudio.org/ticket/158#comment:1>
PulseAudio <http://pulseaudio.org/>
The PulseAudio Sound Server