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

[Nikolay Gorshkov]

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
>>
>