Leaked file descriptor when nsISound.play is given a file type it can't handle

NEW
Unassigned

Status

()

Core
Widget
4 years ago
4 years ago

People

(Reporter: njn, Unassigned)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

4 years ago
My (non-e10s) trunk builds have been flaky lately. Typically what happens is
about 4--6 hours into the session, when I try to use it after not having
touched it for a while, I get one of the following two behaviours:

- A crash.

- Weird drawing artifacts -- lots of things don't render properly, including
  web content and menus, which makes the browser unusable.

It just happened now and I got this output, variants of which I've seen before:

> console.error: 
>   Could not write session state file 
>   Message: Unix error 24 during operation open on file /home/njn/.mozilla/firefox/aijnow0u.default/sessionstore-backups/recovery.js.tmp (Too many open files)
>   
> console.error: 
>   Could not write session state file 
>   Message: Unix error 24 during operation open on file /home/njn/.mozilla/firefox/aijnow0u.default/sessionstore-backups/recovery.js.tmp (Too many open files)
> 
> (firefox:3087): Gtk-WARNING **: Error loading theme icon 'document-open' for stock: Error opening file: Too many open files
> 
> (firefox:3087): Gtk-WARNING **: Error loading theme icon 'window-close' for stock: Error opening file: Too many open files
> 
> (firefox:3087): Gtk-WARNING **: Error loading theme icon 'document-save-as' for stock: Error opening file: Too many open files
> 
> (firefox:3087): Gtk-WARNING **: Error loading theme icon 'document-print-preview' for stock: Error opening file: Too many open files
> 
> (firefox:3087): Gtk-WARNING **: Error loading theme icon 'document-print' for stock: Error opening file: Too many open files
> 
> (firefox:3087): Gtk-WARNING **: Error loading theme icon 'application-exit' for stock: Error opening file: Too many open files
> console.error: 
>   Could not write session state file 
>   Message: Unix error 24 during operation open on file /home/njn/.mozilla/firefox/aijnow0u.default/sessionstore-backups/recovery.js.tmp (Too many open files)
>   
> console.error: 
>   Could not write session state file 
>   Message: Unix error 24 during operation open on file /home/njn/.mozilla/firefox/aijnow0u.default/sessionstore.js.tmp (Too many open files)

Perhaps we have an fd leak. I didn't manage to look at /proc/<pid>/fd before
the session ended. I'll try to do that next time.
(Reporter)

Comment 1

4 years ago
It happened again and /proc/<pid>/fd tells me that it's ChatZilla's fault. I had an mp3 file set to play when the "normal chat" and "stalk match" sound events occurred, and I was up to 999 open file descriptors, and most of them were for the mp3 file.
(Reporter)

Comment 2

4 years ago
Also, ironically enough, I was never able to get the mp3 sound files to actually trigger in Chatzilla. Perhaps there's a single underlying problem causing both symptoms -- the files can't play and as a result they stay open, or something.
Assignee: nobody → rginda
Component: General → ChatZilla
Product: Firefox → Other Applications
Summary: Drawing glitches and crashes due to "Too many open files" → Drawing glitches and crashes due to "Too many open files", due to Chatzilla not closing sound event files
Version: 29 Branch → unspecified
(Reporter)

Comment 3

4 years ago
This is with Chatzilla 0.9.90.1, on Ubuntu 14.04.

Comment 4

4 years ago
(In reply to Nicholas Nethercote [:njn] from comment #3)
> This is with Chatzilla 0.9.90.1, on Ubuntu 14.04.

CZ uses nsISound, so I fear we'll just punt this back to gecko:

http://mxr.mozilla.org/comm-central/source/mozilla/extensions/irc/xul/content/static.js#1484

Presumably, if you run something like this in the browser console / a browser scratchpad, the same happens:

let s = Cc["@mozilla.org/sound;1"].createInstance(Ci.nsISound);
for (let i = 0; i < 1000; i++) {
  s.play(makeURI("file:///path/to/your/favourite/mp3/sound"));
}


?
Assignee: rginda → nobody
Component: ChatZilla → Widget
Flags: needinfo?(n.nethercote)
Product: Other Applications → Core
Summary: Drawing glitches and crashes due to "Too many open files", due to Chatzilla not closing sound event files → Drawing glitches and crashes due to "Too many open files", due to nsISound.play not closing mp3 files when they are unplayable
Version: unspecified → Trunk
(Reporter)

Comment 5

4 years ago
The exact code ended up being the following, and focus had to be in a chrome tab for it to run correctly:

> Cu.import("resource://services-common/utils.js");
> let s = Components.classes["@mozilla.org/sound;1"].createInstance(Ci.nsISound);
> for (let i = 0; i < 1000; i++) {
>   s.play(CommonUtils.makeURI("file:///home/njn/Downloads/Robot-blip.mp3"));
> }

Yes, this recreated the problem -- no sound was played, but I ended up with all the open file descriptors.

In the title you used the word "unplayable" -- but if I enter file:///home/njn/Downloads/Robot-blip.mp3 into the address bar in a new tab the mp3 plays just fine.
Flags: needinfo?(n.nethercote)
(Reporter)

Comment 6

4 years ago
I added some debugging printfs in widget/gtk/nsSound.cpp. nsSound::Play() is running successfully, not bailing on any exit paths, but doesn't produce any sound.

nsSound::Beep() also runs seemingly successfully, but I can't get a beep sound either.
(Reporter)

Comment 7

4 years ago
If I run this command:

> canberra-gtk-play -f ~/Downloads/Robot-blip.mp3

I get this output:

> Failed to play sound: File or data corrupt

Can libcanberra play MP3 files?
(Reporter)

Comment 8

4 years ago
> Can libcanberra play MP3 files?

It cannot. However, it can play both .ogg and .wav files. I have to also use the -V option to get a sound out of canberra-gtk-play. So that's progress of a sort, but nsSound::Play() still doesn't produce any sound.
(Reporter)

Comment 9

4 years ago
kinetik helped me work out what's happening.

- nsSound::Play() is working with a .ogg file. The volume on my monitor was really low so I couldn't hear it. (For some strange reason when playing it via HTML the volume was higher. Not sure why, but not relevant for this bug.)

- nsSound::Play() fails with a .mp3 file. The call to ca_context_play() returns CA_ERROR_CORRUPT -- which matches the "Failed to play sound: File or data corrupt" I get with canberra-gtk-play -- but that return value is ignored.
Summary: Drawing glitches and crashes due to "Too many open files", due to nsISound.play not closing mp3 files when they are unplayable → Leaked file descriptor when nsISound.play is given a file type it can't handle
On successfully opening a Wave, ca_wav_close will close the FILE*.  For Vorbis, ov_clear does the same.  So it looks like the leak arises because ca_sound_file_open forgets to call fclose() on failure.  I filed a bug against libcanberra upstream: https://bugs.freedesktop.org/show_bug.cgi?id=83959
Thinking about what we can do until a fixed libcanberra is available: we can assume the leak has happened if ca_context_play returns CA_ERROR_CORRUPT, but the FILE* has been lost on the stack, so even if we dig the file descriptor out of /proc/self/fds and close it, we'd leak the FILE*.

We could use NS_SniffContent to get the MIME type and then reject playback if it's not APPLICATION_OGG or AUDIO_WAV, but the best that'd do is reduce the chance of leaking somewhat as it's still possible that the Ogg or Wave is undecodable for various reasons.

Comment 12

4 years ago
(In reply to Matthew Gregan [:kinetik] from comment #11)
> Thinking about what we can do until a fixed libcanberra is available: we can
> assume the leak has happened if ca_context_play returns CA_ERROR_CORRUPT,
> but the FILE* has been lost on the stack, so even if we dig the file
> descriptor out of /proc/self/fds and close it, we'd leak the FILE*.
> 
> We could use NS_SniffContent to get the MIME type and then reject playback
> if it's not APPLICATION_OGG or AUDIO_WAV, but the best that'd do is reduce
> the chance of leaking somewhat as it's still possible that the Ogg or Wave
> is undecodable for various reasons.

Does canberra not expose any other method than play() which tells you if canberra *can* actually play it, that doesn't leak the file pointer?

In the meantime, can consumers of nsISound work around by creating new instances and having the old ones be GC'd? I guess not?
Flags: needinfo?(kinetik)

Comment 13

4 years ago
Alternatively, I wonder if for better support of various formats and so on, we shouldn't just make nsISound use the same backend as HTML5 <audio>, which will get us mp3 support "for free" on many platforms (including linux if compiled with gstreamer, I think?).
(In reply to :Gijs Kruitbosch (Mostly gone until 22/9) from comment #12)
> Does canberra not expose any other method than play() which tells you if
> canberra *can* actually play it, that doesn't leak the file pointer?

Not that I'm aware of, but I'm not overly familiar with it.  The API is intentionally fairly simple: https://developer.gnome.org/libcanberra/unstable/libcanberra-canberra.html

> In the meantime, can consumers of nsISound work around by creating new
> instances and having the old ones be GC'd? I guess not?

Sadly not.

(In reply to :Gijs Kruitbosch (Mostly gone until 22/9) from comment #13)
> Alternatively, I wonder if for better support of various formats and so on,
> we shouldn't just make nsISound use the same backend as HTML5 <audio>, which
> will get us mp3 support "for free" on many platforms (including linux if
> compiled with gstreamer, I think?).

That's what bug 683263 proposes.
Flags: needinfo?(kinetik)
Couple of other potential workarounds:

- fork a new process to play the sound (insanely heavyweight, but does "solve" the leak)

- return an error from play() if we get CA_ERROR_CORRUPT and fix all the callers so that they can inform the user/disable the sound (minimizes the leak to once per sound file, more or less)
(Reporter)

Comment 16

4 years ago
> - fork a new process to play the sound (insanely heavyweight, but does
> "solve" the leak)

Upstreaming the trivial fix and then getting that imported into Firefox is the best solution, right? Is this a significant enough bug that it's worth going to great efforts to work around it in the meantime?
(In reply to Nicholas Nethercote [:njn] from comment #16)
> Upstreaming the trivial fix and then getting that imported into Firefox is
> the best solution, right? Is this a significant enough bug that it's worth
> going to great efforts to work around it in the meantime?

We don't ship libcanberra, so we'd either have to start or wait for distros to pick up the fix.

But yeah, given the situation, it's not a bug I plan on expending any further effort on.
You need to log in before you can comment on or make changes to this bug.