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

Thu Mar 3 13:34:24 UTC 2016

I’d like to cast a vote for inclusion this fix in jdk9.

Probably it has to be done in the original review thread created by Alex:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036463.html
http://mail.openjdk.java.net/pipermail/jdk9-dev/2015-November/003036.html
but there is no any activity from November 2015…

So, +1 to get this fix in jdk9.

Thanks,
Andrew

On Feb 24, 2016, at 5:07 PM, Dmitry Cherepanov <dcherepanov at azul.com<mailto:dcherepanov at azul.com>> wrote:

On Feb 19, 2016, at 1:55 PM, Nikolay Gorshkov <nikolay at azulsystems.com<mailto:nikolay at azulsystems.com>> 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

I’m not expert in this area but the changes look reasonable to me. +1 for pushing this into JDK9.

Thanks

Dmitry

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