Skip to main content

MMAPI BasicPlayer Leak

5 replies [Last post]
Anonymous

I wanted to get some wider opinion on something I noticed not too long
ago.

Something I noticed while I was reading the recently released phoneme
mr1 source code is that there is a private static hashtable field in the
com.sun.mmedia.BasicPlayer class called mplayers. It seems that once you
create a Player it will be added to that hash table with a unique id
(from a hardcoded pool of 32768 ids) and it won't be released from the
hash table until you call the close() method.

A long time ago I used to play sounds literally using the most basic
implementation imaginable, just see the Player Javadocs and look for the
"Simple Playback Example". If what I am reading in BasicPlayer class is
true for actual devices this example would result in a memory leak.

Has anyone else noticed this issue? It seems like the Javadocs are
silent on the issue. Their simple example would cause a lot of
developers to end up leaking memory and they never explicitly explain
that you must close a Player or it will never be garbage collected (well
at least until you happen to make 32768 Players). Seems like the
Javadocs should mention this somewhere unless I am off in my analysis of
the BasicPlayer class.

Jacob

===========================================================================
To unsubscribe, send email to listserv@java.sun.com and include in the body
of the message "signoff KVM-INTEREST". For general help, send email to
listserv@java.sun.com and include in the body of the message "help".

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
Joe Bowbeer

I agree that the basic example is incomplete. All Players should be
closed (or at least deallocated). A PlayerListener is required to do
this properly. I recommend using a PlayerListener in even the most
basic application. I think the Player documentation should show this
as well.

Here's a template:

Player playSound(InputStream in, String mime)
throws MediaException {
Player player = Manager.createPlayer(in, mime);
player.addPlayerListener(this);
player.start();
return player;
}

void stopSound(Player player) throws MediaException {
try {
player.stop();
} catch (IllegalStateException ex) {
// already closed
}
}

/** PlayerListener implementation. */
public void playerUpdate(Player player, String event, Object eventData) {
if (event.equals(PlayerListener.STARTED)) {
// player started
} else if (
event.equals(PlayerListener.END_OF_MEDIA) ||
event.equals(PlayerListener.STOPPED)) {
try {
player.removePlayerListener(this);
player.close();
} catch (IllegalStateException ex) {
// already closed
}
// player closed
} else if (event.equals(PlayerListener.ERROR)) {
// player error
}
}

On 5/29/07, Jacob Abrams wrote:
> I wanted to get some wider opinion on something I noticed not too long
> ago.
>
> Something I noticed while I was reading the recently released phoneme
> mr1 source code is that there is a private static hashtable field in the
> com.sun.mmedia.BasicPlayer class called mplayers. It seems that once you
> create a Player it will be added to that hash table with a unique id
> (from a hardcoded pool of 32768 ids) and it won't be released from the
> hash table until you call the close() method.
>
> A long time ago I used to play sounds literally using the most basic
> implementation imaginable, just see the Player Javadocs and look for the
> "Simple Playback Example". If what I am reading in BasicPlayer class is
> true for actual devices this example would result in a memory leak.
>
> Has anyone else noticed this issue? It seems like the Javadocs are
> silent on the issue. Their simple example would cause a lot of
> developers to end up leaking memory and they never explicitly explain
> that you must close a Player or it will never be garbage collected (well
> at least until you happen to make 32768 Players). Seems like the
> Javadocs should mention this somewhere unless I am off in my analysis of
> the BasicPlayer class.
>
> Jacob
>

===========================================================================
To unsubscribe, send email to listserv@java.sun.com and include in the body
of the message "signoff KVM-INTEREST". For general help, send email to
listserv@java.sun.com and include in the body of the message "help".

C. Enrique Ortiz

Yes, always implement a player listener per Joe... I also would like to
say that it is a good idea also to _always deallocate_ the player when
done -- some handsets will just choke when trying to play (even a
new/different player instance, or different sound asset), if the
previous player was not deallocated...

:
} else if (event.equals(PlayerListener.END_OF_MEDIA)) {
try {
player.stop();
player.deallocate();
player.close();
} catch (MediaException me) {
....
} catch (IOException ioe) {
....
}
}

ceo

--
C. Enrique Ortiz
http://www.CEnriqueOrtiz.com
http://www.CEnriqueOrtiz.com/weblog
http://www.MobileMondayAustin.org
http://www.AustinWirelessAlliance.org

Joe Bowbeer wrote:
> I agree that the basic example is incomplete. All Players should be
> closed (or at least deallocated). A PlayerListener is required to do
> this properly. I recommend using a PlayerListener in even the most
> basic application. I think the Player documentation should show this
> as well.
>
> Here's a template:
>
> Player playSound(InputStream in, String mime)
> throws MediaException {
> Player player = Manager.createPlayer(in, mime);
> player.addPlayerListener(this);
> player.start();
> return player;
> }
>
> void stopSound(Player player) throws MediaException {
> try {
> player.stop();
> } catch (IllegalStateException ex) {
> // already closed
> }
> }
>
> /** PlayerListener implementation. */
> public void playerUpdate(Player player, String event, Object
> eventData) {
> if (event.equals(PlayerListener.STARTED)) {
> // player started
> } else if (
> event.equals(PlayerListener.END_OF_MEDIA) ||
> event.equals(PlayerListener.STOPPED)) {
> try {
> player.removePlayerListener(this);
> player.close();
> } catch (IllegalStateException ex) {
> // already closed
> }
> // player closed
> } else if (event.equals(PlayerListener.ERROR)) {
> // player error
> }
> }
>
>
> On 5/29/07, Jacob Abrams wrote:
>> I wanted to get some wider opinion on something I noticed not too long
>> ago.
>>
>> Something I noticed while I was reading the recently released phoneme
>> mr1 source code is that there is a private static hashtable field in the
>> com.sun.mmedia.BasicPlayer class called mplayers. It seems that once you
>> create a Player it will be added to that hash table with a unique id
>> (from a hardcoded pool of 32768 ids) and it won't be released from the
>> hash table until you call the close() method.
>>
>> A long time ago I used to play sounds literally using the most basic
>> implementation imaginable, just see the Player Javadocs and look for the
>> "Simple Playback Example". If what I am reading in BasicPlayer class is
>> true for actual devices this example would result in a memory leak.
>>
>> Has anyone else noticed this issue? It seems like the Javadocs are
>> silent on the issue. Their simple example would cause a lot of
>> developers to end up leaking memory and they never explicitly explain
>> that you must close a Player or it will never be garbage collected (well
>> at least until you happen to make 32768 Players). Seems like the
>> Javadocs should mention this somewhere unless I am off in my analysis of
>> the BasicPlayer class.
>>
>> Jacob
>>
>
> ===========================================================================
>
> To unsubscribe, send email to listserv@java.sun.com and include in the
> body
> of the message "signoff KVM-INTEREST". For general help, send email to
> listserv@java.sun.com and include in the body of the message "help".

===========================================================================
To unsubscribe, send email to listserv@java.sun.com and include in the body
of the message "signoff KVM-INTEREST". For general help, send email to
listserv@java.sun.com and include in the body of the message "help".
[att1.html]

Joe Bowbeer

On 5/29/07, C. Enrique Ortiz wrote:
>
> I also would like to say that it is a good idea also to always deallocate
> the player when done -- some handsets will just choke when trying to
> play (even a new/different player instance, or different sound asset),
> if the previous player was not deallocated...
>
> } else if (event.equals(PlayerListener.END_OF_MEDIA)) {
> try {
> player.stop();
> player.deallocate();
> player.close();
> } catch (MediaException me) {
> ....
> } catch (IOException ioe) {
> ....
> }
> }
>

Good point that many handsets only support a single active Player.
(Some will support multiple players if the formats are different,
e.g., mp3 and midi, and newer handsets may support multiple players of
the same type.)

However, player.close() is supposed to deallocate so I'm confused
about the explicit call to deallocate. (I'm also confused about the
IOException.)

In the simplest case, where the listener's job is mainly to cleanup so
that someone else can play, I've found it easiest to handle the
END_OF_MEDIA and STOPPED events the same, by calling close().

I also like to avoid some confusion by removing the listener before
calling close().

Rationale: the player will be CLOSED when close() returns, the
listener can't be removed after the player is CLOSED
(IllegalStateException), and the events sent while close() is in
progress do not concern me anymore.

} else if (
event.equals(PlayerListener.END_OF_MEDIA) ||
event.equals(PlayerListener.STOPPED)) {
try {
player.removePlayerListener(this);
player.close();
} catch (IllegalStateException ex) {
// already closed
}
}

--
Joe Bowbeer

===========================================================================
To unsubscribe, send email to listserv@java.sun.com and include in the body
of the message "signoff KVM-INTEREST". For general help, send email to
listserv@java.sun.com and include in the body of the message "help".

C. Enrique Ortiz

See below...

ceo

Joe Bowbeer wrote:
> On 5/29/07, C. Enrique Ortiz wrote:
>>
>> I also would like to say that it is a good idea also to always
>> deallocate
>> the player when done -- some handsets will just choke when trying to
>> play (even a new/different player instance, or different sound asset),
>> if the previous player was not deallocated...
>>
>> } else if (event.equals(PlayerListener.END_OF_MEDIA)) {
>> try {
>> player.stop();
>> player.deallocate();
>> player.close();
>> } catch (MediaException me) {
>> ....
>> } catch (IOException ioe) {
>> ....
>> }
>> }
>>
>
> Good point that many handsets only support a single active Player.
> (Some will support multiple players if the formats are different,
> e.g., mp3 and midi, and newer handsets may support multiple players of
> the same type.)
>
> However, player.close() is supposed to deallocate so I'm confused
> about the explicit call to deallocate. (I'm also confused about the
> IOException.)
ceo: But some (older) handsets don't. About the catch for IOException,
it is there because of a stream that I close (in my original code); I
posted a modified version on which I removed the stream to keep it
simple - but forgot to remove the IOException catch... Good eye ;-)

> In the simplest case, where the listener's job is mainly to cleanup so
> that someone else can play, I've found it easiest to handle the
> END_OF_MEDIA and STOPPED events the same, by calling close().
ceo: Yes...
>
> I also like to avoid some confusion by removing the listener before
> calling close().
ceo: Yes, it is a good practice to do so...
>
> Rationale: the player will be CLOSED when close() returns, the
> listener can't be removed after the player is CLOSED
> (IllegalStateException), and the events sent while close() is in
> progress do not concern me anymore.
>
> } else if (
> event.equals(PlayerListener.END_OF_MEDIA) ||
> event.equals(PlayerListener.STOPPED)) {
> try {
> player.removePlayerListener(this);
> player.close();
> } catch (IllegalStateException ex) {
> // already closed
> }
> }
ceo: Yes.

Thanks...
ceo
>
> --
> Joe Bowbeer
>
> ===========================================================================
>
> To unsubscribe, send email to listserv@java.sun.com and include in the
> body
> of the message "signoff KVM-INTEREST". For general help, send email to
> listserv@java.sun.com and include in the body of the message "help".
>
> --------------------------------
> Spam/Virus scanning by CanIt Pro
>
> For more information see
> http://www.kgbinternet.com/SpamFilter.htm
>
> To control your spam filter, log in at
> http://filter.kgbinternet.com
>

--
C. Enrique Ortiz
eortiz@j2medeveloper.com
512-635-4225
http://www.CEnriqueOrtiz.com
http://www.CEnriqueOrtiz.com/weblog
http://www.MobileMondayAustin.org
http://www.AustinWirelessAlliance.org

===========================================================================
To unsubscribe, send email to listserv@java.sun.com and include in the body
of the message "signoff KVM-INTEREST". For general help, send email to
listserv@java.sun.com and include in the body of the message "help".

Robin Chaddock

That's not at all suprising.

No doubt Sun wrote the reference implementation that way so as to
highlight the fact that failing to call Player.close() is a very bad
thing to do.

Exactly the same applies to InputStreams and rms access too.

There are no finalizers in cldc, therefor it is impossible for the vm to
dispose of associated native resources when it collects garbage.
Even if there were, it would likely be a very bad way of ensuring Player
closure given the finite nature of the underlying native resources.

Jacob Abrams wrote:
> I wanted to get some wider opinion on something I noticed not too long
> ago.
>
> Something I noticed while I was reading the recently released phoneme
> mr1 source code is that there is a private static hashtable field in the
> com.sun.mmedia.BasicPlayer class called mplayers. It seems that once you
> create a Player it will be added to that hash table with a unique id
> (from a hardcoded pool of 32768 ids) and it won't be released from the
> hash table until you call the close() method.
>
> A long time ago I used to play sounds literally using the most basic
> implementation imaginable, just see the Player Javadocs and look for the
> "Simple Playback Example". If what I am reading in BasicPlayer class is
> true for actual devices this example would result in a memory leak.
>
> Has anyone else noticed this issue? It seems like the Javadocs are
> silent on the issue. Their simple example would cause a lot of
> developers to end up leaking memory and they never explicitly explain
> that you must close a Player or it will never be garbage collected (well
> at least until you happen to make 32768 Players). Seems like the
> Javadocs should mention this somewhere unless I am off in my analysis of
> the BasicPlayer class.
>
> Jacob
>
> ===========================================================================
> To unsubscribe, send email to listserv@java.sun.com and include in the body
> of the message "signoff KVM-INTEREST". For general help, send email to
> listserv@java.sun.com and include in the body of the message "help".
>

________________________________________________________________________
E-mail is an informal method of communication and may be subject to data corruption, interception and unauthorised amendment for which I-play, a trading name of Digital Bridges Ltd will accept no liability. Therefore, it will normally be inappropriate to rely on information contained on e-mail without obtaining written confirmation.

This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and destroy this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

(C) 2005. I-play is a trademark and trading name of Digital Bridges Limited. All Rights Reserved.
________________________________________________________________________
This message has been checked for all known viruses by the
MessageLabs Virus Scanning Service. For further information visit
http://www.messagelabs.com/stats.asp

===========================================================================
To unsubscribe, send email to listserv@java.sun.com and include in the body
of the message "signoff KVM-INTEREST". For general help, send email to
listserv@java.sun.com and include in the body of the message "help".