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

[Alex Kashchenko]

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