Permgen issues when using Gradle daemon about asciidoctor-gradle-plugin

[rwinch]

Detailed Explanation of the problem

My suspicion was correct. A detailed explanation of the issue is that JavaUtilLoggingLogger was doing the following:

• Creating an anonymous inner class of Formatter
• Since the Formatter was created with a custom ClassLoader (i.e. Gradle's MutableURLClassLoader) that class has a reference (i.e. Formatter.class.classLoader) to the custom ClassLoader
• A reference to that Formatter was then added to a java.util.logging.Logger instance which is statically cached by the system ClassLoader
• This means that the custom ClassLoader had a GC root within the system ClassLoader's static cache of the Logger instance and could not be GC'd.

Since the custom ClassLoader could not be GC'd then all the classes that were created by it (i.e class within the Gradle build) could not be GC'd either since each ClassLoader caches the classes it loads. This is what was causing the Perm Gen leak.

Of course depending on various code paths there could be other leaks, but this was the first one that I encountered.

Solution / Workaround

I think eventually we need to log a proper issue with JRuby team and have them remove the anonymous inner class. For now we can work around this by setting the following system property:

System.setProperty('jruby.logger.class','org.jruby.util.log.StandardErrorLogger')

For those experiencing the issue, they should be able to work around it by using the following doFirst closure:

asciidoctor {
    // ...
    doFirst {
        System.setProperty('jruby.logger.class','org.jruby.util.log.StandardErrorLogger')
    }
}

[lordofthejars]

Cool, if this is a problem which potentially can affect all users of AsciidoctorJ, not only the Gradle ones, then we could predefine this value during AsciidoctorJ startup time. For what I see this is a problem with JRuby first, but only affects Gradle users for now, but I am not sure that this could affect OSGi users too for example.

[rwinch]

@lordofthejars

I fully agree this is primarily a JRuby issue. I created an issue and Pull Request with JRuby.

This issue impacts users of JRuby that are running on a application server that do not restart the container between deploys of the application. In that instance the leak would occur due to the war's ClassLoader. Therefore, if we are wanting to provide a workaround by setting the system property (please let me know your thoughts here) I believe it makes the most sense to set the system property in AsciidoctorJ.

[lordofthejars]

Hi thank you so much for your research.
I think you have done a really good work, about next steps I think it would
depend on what JRuby folks will do. Do you think they will spend so much
time in merging and releasing a new version? If they act fast then we can
wait, if not we open an issue to asciidoctorj to create the workaround

El dilluns 25 de novembre de 2013, Rob Winch ha escrit:

@lordofthejars https://github.com/lordofthejars

I fully agree this is primarily a JRuby issue. I created an issuehttps://github.com/jruby/jruby/issues/1266and Pull
Request jruby/jruby#1267 with JRuby.

This issue impacts users of JRuby that are running on a application server
that do not restart the container between deploys of the application. In
that instance the leak would occur due to the war's ClassLoader. Therefore,
if we are wanting to provide a workaround by setting the system property
(please let me know your thoughts here) I believe it makes the most sense
to set the system property in AsciidoctorJ.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/61#issuecomment-29220543
.

Enviat amb Gmail Mobile

[rwinch]

I'm not really certain how fast we will see it merged and released. I am fairly hopeful though since I have already gotten a +1 from one of the JRuby comitters to merge my PR

[lordofthejars]

Cool then for now and because users can still add the property by
themselves (as workaround of course) we can left as is and wait. If it
takes so time, then I will add to asciidoctorj. Has it sense?

2013/11/26 Rob Winch [email protected]

I'm not really certain how fast we will see it merged and released. I am
fairly hopeful though since I have already gotten a +1 from one of the
JRuby comitters to merge my PR

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/61#issuecomment-29301646
.

+----------------------------------------------------------+
Alex Soto Bueno - Computer Engineer
www.lordofthejars.com
+----------------------------------------------------------+

[rwinch]

That works for me :)

[mojavelinux]

+1

Thanks for tracking this down Rob! You nailed down the problem before it
even had a chance to cause too much disruption for users. \o/

I'll help lobby to get the patch merged.

Cheers!

-Dan

[lordofthejars]

Cool, as I suggested to Rob, if it takes more time that expected, I will patch AsciidoctorJ as a temporary workaround.

[aalmiray]

Given the last exchange, should be close this issue and open another @ AsciidoctorJ ?

[lordofthejars]

Well It should be fixed in AsciidoctorJ as well asciidoctor/asciidoctorj#96 In next version you can remove the clone of the internal Map.

[aalmiray]

Do we have a test case for this? Methinks this issue can be closed due to asciidoctorj-1.5.2 depends on JRuby 1.7.16.1

[rwinch]

It appears to have been resolved. Sorry I didn't mean to click close. I will leave that up to @aalmiray

[aalmiray]

no worries @rwinch 😄