[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-distro-pkg-dev
Subject:    RFC: PR1050 - Stream objects not garbage collected?
From:       ahughes () redhat ! com (Andrew Hughes)
Date:       2012-06-28 23:20:09
Message-ID: d92e15ae-7ad7-41bc-bde0-2e4c00eeef6d () zmail17 ! collab ! prod ! int ! phx2 ! redhat ! com
[Download RAW message or body]

----- Original Message -----
> On 06/27/2012 07:26 PM, Andrew Hughes wrote:
> > 
> > 
> > ----- Original Message -----
> >> Hi,
> >>
> >> The attached patch is a fix for PR105. It was caused by the JVM
> >> hanging
> >> on to global references to Stream objects. A test case is included
> >> (causes an OutOfMemoryError without the fix). I did notice a few
> >> other
> >> structs not being free()d and removed them (or added calls to
> >> free()
> >> them).
> >>
> >> Okay to commit?
> >>
> >> ChangeLog:
> >> 2012-05-27  Omair Majid  <omajid at redhat.com>
> >>
> >> *
> >> pulseaudio/src/java/org/classpath/icedtea/pulseaudio/Stream.java:
> >> Add new member variable contextPointer.
> >> * pulseaudio/src/native/org_classpath_icedtea_pulseaudio_Stream.c
> >> (Java_org_classpath_icedtea_pulseaudio_Stream_native_1pa_1stream_1new):
> >> Save j_context as contextPointer.
> >> (Java_org_classpath_icedtea_pulseaudio_Stream_native_1pa_1stream_1unref):
> >> Delete the global ref and dellocate the java context
> >> (cork_callback): Don't check userdata. It is NULL.
> >> (Java_org_classpath_icedtea_pulseaudio_Stream_native_1pa_1stream_1cork):
> >> Dont allocate and pass a java_context to pa_stream_cork. It is not
> >> needed.
> >> *
> >> pulseaudio/unittests/org/classpath/icedtea/pulseaudio/PulseAudioClipTest.java
> >> (testOpenCloseLotsOfTimes): New method.
> >>
> >> Thanks,
> >> Omair
> >>
> > 
> > Please do to 6, 7, 8.
> 
> I am going to hold off on 8 for the moment. It needs a repeat of
> forward-port of the pulse-java fixes that I did from 6 to 7 (this
> time 6
> to 8) first. Instead, I am going to try and see if this can be
> included
> in upstream jdk8 itself.
> 

I don't think taking it upstream to OpenJDK8 is the right approach.
It won't solve this problem (fixes will still need to go across at
least three trees).  In fact, it'll make things harder as we'll then
need to get reviewer approval and an Oracle bug ID for every fix.

I think a better approach would be to reinvigorate the IcedTea PulseAudio
repository or, preferably, setup an upstream OpenJDK project for it,
as you suggested on IRC.  Having it tied to specific JDK versions is a
maintenance nightmare.

I believe IcedTea-Web has worked out well under its current model and
the PulseAudio addon would do well to follow in its footsteps.

> >  Did you have any branches in mind too?
> 
> Not at the moment. I know the bug was filed against 1.9, but that is
> missing other pulse-java fixes too. Let me ask the reporter about
> this.

1.9 is no longer supported, so they should be told to upgrade to at least
1.10.x

> 
> Thanks,
> Omair
> 

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07


[prev in list] [next in list] [prev in thread] [next in thread]