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

[Nikolay Gorshkov]

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

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