RFR [7] 8133206: "java.lang.OutOfMemoryError: unable to create new native thread" caused by upgrade to zlib 1.2.8

[Alex Kashchenko]

Hi,

On 02/19/2016 10:55 AM, Nikolay Gorshkov wrote:
> Hi Sherman, Sean,
>
> Could you please help with making progress on this code review request?
> This fix is waiting for review since October.
>
> Webrev for jdk7u:
> http://cr.openjdk.java.net/~nikgor/8133206/jdk7u-dev/webrev.01/
> Original mail thread:
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-October/035884.html
>
>
> Webrev for jdk9 (contributed by Alex Kashchenko):
> http://cr.openjdk.java.net/~akasko/jdk9/8133206/webrev.00/
> Original mail thread:
> http://mail.openjdk.java.net/pipermail/jdk9-dev/2015-November/003036.html

Just a note on a jtreg reproducer - please let me know if any 
changes/improvements will be needed for it.

>
> Thanks,
> Nikolay
>
> On 11.12.2015 15:09, Nikolay Gorshkov wrote:
>> Hi Sean,
>>
>> Thank you for your attention to this matter!
>> Actually, the code review request was sent to core-libs-dev alias a
>> month ago:
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036463.html
>>
>> Unfortunately, we haven't got any feedback yet.
>>
>> Thanks,
>> Nikolay
>>
>> On 11.12.2015 14:34, Seán Coffey wrote:
>>> Hi Alex,
>>>
>>> I'm dropping jdk7u-dev mailing list for the moment. core-libs-dev is the
>>> mailing list where this review should happen. This fix would be
>>> required in JDK
>>> 9 first as per process. I think Sherman would be best to review if
>>> possible.
>>>
>>> Once it's soaked in JDK 9 for a few weeks, we could consider jdk8u
>>> and 7u
>>> backports.
>>>
>>> Regards,
>>> Sean.
>>>
>>> On 10/11/15 10:57, Alex Kashchenko wrote:
>>>> Hi,
>>>>
>>>> On 10/21/2015 04:39 PM, Nikolay Gorshkov wrote:
>>>>> Hi Sherman,
>>>>>
>>>>> Thank you for your reply! My answers are inlined.
>>>>>
>>>>>  > Can you be more specific about the "class loading cases" above?
>>>>> Sounds
>>>>>  > more like we have a memory leaking here (the real root cause)? for
>>>>> example
>>>>>  > the inflateEnd() never gets called?
>>>>>
>>>>> I agree, the real root cause is probably the following issue that
>>>>> exists
>>>>> since the end of 2002:
>>>>> https://bugs.openjdk.java.net/browse/JDK-4797189
>>>>> "Finalizers not called promptly enough"
>>>>> And it is "the absence of a general solution to the non-heap resource
>>>>> exhaustion problem".
>>>>>
>>>>> zlib's inflateEnd() function is called by
>>>>> void java.util.zip.Inflater.end(long addr)
>>>>> native method only, and this method, in turn, is called only by
>>>>> void java.util.zip.Inflater.end()
>>>>> and
>>>>> void java.util.zip.Inflater.finalize()
>>>>> methods. According to the experiments, the typical stack trace for
>>>>> instantiating java.util.zip.Inflater is:
>>>>>
>>>>> java.util.zip.Inflater.<init>(Inflater.java:116)
>>>>> java.util.zip.ZipFile.getInflater(ZipFile.java:450)
>>>>> java.util.zip.ZipFile.getInputStream(ZipFile.java:369)
>>>>> java.util.jar.JarFile.getInputStream(JarFile.java:412)
>>>>> org.jboss.virtual.plugins.context.zip.ZipFileWrapper.openStream(ZipFileWrapper.java:222)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>    <more jboss frames>
>>>>> org.jboss.classloader.spi.base.BaseClassLoader$2.run(BaseClassLoader.java:592)
>>>>>
>>>>>
>>>>> java.security.AccessController.doPrivileged(Native Method)
>>>>> org.jboss.classloader.spi.base.BaseClassLoader.loadClassLocally(BaseClassLoader.java:591)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>    <more jboss frames>
>>>>> org.jboss.classloader.spi.base.BaseClassLoader.loadClass(BaseClassLoader.java:447)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> java.lang.ClassLoader.loadClass(ClassLoader.java:358)
>>>>> java.lang.Class.forName0(Native Method)
>>>>> java.lang.Class.forName(Class.java:278)
>>>>> org.jboss.deployers.plugins.annotations.WeakClassLoaderHolder.loadClass(WeakClassLoaderHolder.java:72)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>    <more jboss and other frames>
>>>>>
>>>>> It's quite hard to understand who is responsible for not calling
>>>>> Inflater.end()
>>>>> method explicitly; probably, it is the jboss/application's code.
>>>>> Unfortunately,
>>>>> we were in "it worked before and is broken now" customer situation
>>>>> here, so
>>>>> needed to fix it anyway.
>>>>>
>>>>>  >  From the doc/impl in inflate() it appears the proposed change
>>>>> should be
>>>>>  > fine, though it's a little hacky, as you never know if it starts to
>>>>> return
>>>>>  > Z_OK from some future release(s). Since the "current"
>>>>> implementation
>>>>>  > never returns Z_OK, it might be worth considering to keep the
>>>>> Z_OK logic
>>>>>  > asis in Inflater.c, together with the Z_BUF_ERROR, just in case?
>>>>>
>>>>> OK, I added handling of Z_OK code back.
>>>>>
>>>>>  > I would be desired to add some words in Inflater.c to remind the
>>>>>  > future maintainer why we switched from partial to finish and why to
>>>>>  > check z_buf_error.
>>>>>
>>>>> I agree, added a comment.
>>>>>
>>>>> The updated webrev is available here:
>>>>>
>>>>> http://cr.openjdk.java.net/~nikgor/8133206/jdk7u-dev/webrev.01/
>>>>>
>>>>
>>>> The change looks good to me (not a Reviewer/Committer).
>>>>
>>>> Patched jdk7u also passes JCK-7 on RHEL 7.1.
>>>>
>>>> I forward-ported this patch to jdk9 (consulted with Nikolay Gorshkov
>>>> first),
>>>> jtreg reproducer for jdk9 also works with jdk7u -
>>>> http://mail.openjdk.java.net/pipermail/jdk9-dev/2015-November/003036.html
>>>>
>>>>
>>>

-- 
-Alex