Warning about loading tilt/haml in a non thread-safe way

[msgilligan]

I'm seeing this warning in my Gradle-based project:

WARN: tilt autoloading 'tilt/haml' in a non thread-safe way; explicit require 'tilt/haml' suggested.

The same warning also shows up in this sample project: https://github.com/JGrenier/asciidoclet-sample

I'm using Gradle 2.6.

[johncarl81]

Ah, I was able to see the warning you mentioned... Granted, I added my own ./gradlew.sh script and I had to disable the asciidoctor-diagram. Here's the output I got:

asciidoclet-sample$ ./gradlew clean build javadoc
:project:clean
:project:compileJava
:project:processResources UP-TO-DATE
:project:classes
:project:jar
:project:assemble
:project:compileTestJava UP-TO-DATE
:project:processTestResources UP-TO-DATE
:project:testClasses UP-TO-DATE
:project:test UP-TO-DATE
:project:check UP-TO-DATE
:project:build
:project:jrubyPrepareGems
Successfully installed asciidoctor-diagram-1.3.0
Successfully installed asciidoctor-1.5.2
2 gems installed
:project:javadoc
WARN: tilt autoloading 'tilt/haml' in a non thread-safe way; explicit require 'tilt/haml' suggested.
asciidoctor: WARNING: <stdin>: line 16: invalid style for listing block: plantuml
asciidoctor: WARNING: <stdin>: line 24: invalid style for listing block: plantuml
asciidoctor: WARNING: <stdin>: line 32: invalid style for literal block: ditaa

BUILD SUCCESSFUL

@mojavelinux @lordofthejars Any ideas on this one?

[mojavelinux]

This is a meaningless message that tilt issues but unfortunately won't remove. You can get around it by adding tilt to the list of requires (wherever the configuration allows requiring libraries).

[msgilligan]

Do any of you have an example Gradle configuration for AsciiDoclet that does this?

[mojavelinux]

I think it's just something like:

options.requires ['tilt']

or

options.addStringOption '--requires', ['tilt']

I'd like to see this example in the README as it can be useful for using other libraries as well.

[msgilligan]

Neither of those is working for me -- the Gradle script wouldn't even compile. The following compiles:

options.addStringOption '-requires', 'tilt'

...but results in javadoc: error - invalid flag: --requires and a usage message.

[mojavelinux]

This is just a matter of trial and error until we figure out the magic combination of characters necessary to trigger this Doclet option: https://github.com/asciidoctor/asciidoclet/blob/master/src/main/java/org/asciidoctor/asciidoclet/DocletOptions.java#L100-L102

If you can find examples of other parameters / doclets that parallel this...that's probably how I'd start debugging.

[johncarl81]

Just to confirm @mojavelinux as I cant seem to find a way around this warning... we should be requiring tilt via:

asciidoctor.rubyExtensionRegistry().requireLibrary("tilt");

[johncarl81]

BTW @msgilligan, if you want to require multiple libraires, you need to separate them with a comma:

javadoc {
    dependsOn jrubyPrepareGems
    options.docletpath = configurations.asciidoclet.files.asType(List)
    options.doclet = 'org.asciidoctor.Asciidoclet'
    options.overview = "src/main/java/overview.adoc"
    options.addStringOption "-base-dir", "${projectDir}"
    options.addStringOption "encoding", "UTF-8"
    options.addStringOption "-attributes-file", "${projectDir}/config/javadoc-attributes.adoc"
    options.addStringOption "r", "tilt, asciidoctor-diagram"     
    options.addStringOption "-gem-path", System.getenv('GEM_PATH')
    options.addStringOption "encoding","UTF-8"
    options.addStringOption "charset","UTF-8"
    options.addStringOption "-attribute",  "data-uri," +
            "name=${project.name}," +
            "version=${project.version}," +
            "title-link=http://example.com[${project.name} ${project.version}]"
}

-require also works here:

    options.addStringOption "-require", "tilt, asciidoctor-diagram"

[mojavelinux]

I really, really wish Tilt would remove this warning because it is just plain wrong. Autoloading a library is a thread safe operation, so Tilt should not be issuing this warning.

Having said that, I ran across the same problem. Explicitly loading tilt/haml with AsciidoctorJ doesn't make the warning go away...and I'm not sure why that is.

[johncarl81]

Are the tilt guys aware of this? Does this https://github.com/rtomayko/tilt/issues/95 reference the same underlying issue?

[mojavelinux]

Here's the proposed solution. https://github.com/rtomayko/tilt/pull/253 But in general, they seem unwilling to fix it and I've personally moved on from trying to fight for it.

[mojavelinux]

The ironic thing about this warning is that it has nothing to do with whether you explicitly require a library or not. The warning always shows if the Ruby runtime has more than one thread (which is always true for JRuby). The warning is just plain naive.

[mojavelinux]

We should document in the Gradle plugin and Asciidoclet README that this warning can be ignored.

[johncarl81]

Yes, agreed. On Jan 2, 2016 4:55 PM, "Dan Allen" notifications@github.com wrote:

We should document in the Gradle plugin and Asciidoclet README that this warning can be ignored.

— Reply to this email directly or view it on GitHub https://github.com/asciidoctor/asciidoclet/issues/45#issuecomment-168440213 .

[mojavelinux]

Here's why this is a bug in Tilt.

If I use:

Tilt.new 'test.html.haml', 1

...I get the warning.

If I use:

template = Tilt['haml']
template.new 'test.html.haml', 1

...I get the warning.

If I use:

Tilt['haml'].new 'test.html.haml', 1

...I don't get the warning.

I can't make sense of what is going on, but clearly Tilt is confusing itself.

I'm open to changing Asciidoctor to use the last method since that's what gets called under the covers anyway.

[mojavelinux]

Actually, in the end, this looks like a race condition, because it seems to happen sometimes and not others in all three cases.

[mojavelinux]

What's more is that this is a truly naive bug that Tilt is introducing. They are auto-loading a placeholder class, but never unregistering it. So when the autoload mechanism kicks off in Ruby, it sometimes hits the real class and sometimes hits the proxy class. When it hits the proxy, and the Ruby runtime has more than one thread, you see the warning. In the end, though, all roads lead back to the real class, hence why it still functions.

[mojavelinux]

Ironically, both the warning and the content of the warning message are incorrect.

[johncarl81]

Sounds like the real fix is a PR to tilt... If they are open to it. On Jan 2, 2016 5:31 PM, "Dan Allen" notifications@github.com wrote:

Ironically, both the warning and the content of the warning message are incorrect.

— Reply to this email directly or view it on GitHub https://github.com/asciidoctor/asciidoclet/issues/45#issuecomment-168443963 .

[mojavelinux]

Exactly. This really should be solved there because it has to be affecting all JRuby users, especially since the message is happening intermittently.

But now at least we understand what is going on. That's half the battle.

[judofyr]

They are auto-loading a placeholder class, but never unregistering it. So when the autoload mechanism kicks off in Ruby, it sometimes hits the real class and sometimes hits the proxy class.

Hm? What do you mean by this? What is "a placeholder class"? What's the "proxy" object you're talking about later?

[judofyr]

The ironic thing about this warning is that it has nothing to do with whether you explicitly require a library or not.

Ehm, it is. If you require "tilt/haml" first, you won't see the message.

But it should be noted that the warning was added years ago and I don't know the thread-safety of require in the latest Ruby variants. I know for sure it was a problem in the 1.8.7-days.

Considering that no-one actually follows the message in the warning (no-one in this issue mentioned that you should require tilt/haml, just tilt which is not enough), I guess we might as well remove it.

[mojavelinux]

Hey @judofyr!

If you require "tilt/haml" first, you won't see the message.

Nope. We've been able to demonstrate that the message happens intermittently even if tilt/haml is loaded first. (more details on what I mean by the proxy class in a follow-up).

it should be noted that the warning was added years ago and I don't know the thread-safety of require in the latest Ruby variants. I know for sure it was a problem in the 1.8.7-days.

Charles Nutter and Brian Shirai both assured me that require is a thread-safe operation.

...I guess we might as well remove it.

I can attest to the fact that we get numerous support issues about this which ends up burning a lot of support costs...so I'm for just removing the message.

[mojavelinux]

The proxy I'm referring to (really a load proxy, not a proxy class) is the Mapping#template_map. This should be a concurrent map, not a normal Hash (using the thread-safe gem, if available).

Since it is not a concurrent map, Ruby implementations that have multiple threads might pass through here at different times (race condition) and therefore trigger the lazy_load method, which may do a redundant require and therefore issue a warning. (Note that Tilt is not checking the return value of require, so it could be that it was already required, the map was just out of date).

The way it should be implemented is that you attempt a lazy_load if the entry is not in the map, but if you come back with the same result, then you just drop it (multiple workers are going to get the same result) and therefore there is no need for a warning.

Having said all that, I want to admit that I am not an expert on concurrency and therefore, there may be holes in this proposal. What I do know is that a) this warning should only happen if there is really a thread-safety violation and b) it should not happen if the class has already been loaded.

[mojavelinux]

We could always ask Charles, Tom or Brian for a review.

[mojavelinux]

...which is pretty important given that Tilt is one of the top ten gems in the ecosystem.

[mojavelinux]

Here's the test case to demonstrate that it makes no difference if you require 'tilt/haml' first.

test.rb

require 'tilt'
require 'tilt/haml'

template = Tilt['haml']
template.new 'test.html.haml', 1

test.html.haml

p='Hello, World!'

Test run:

rvm use jruby-9.0.4.0@tilt-test --create
for i in {1..10}; do ruby test.rb; done

[mojavelinux]

You'll see that on certain runs, the Thread.list.size is > 1, but at the same time the return value of require is false. What this shows is that library is loaded, but Tilt doesn't have it mapped. So the problem is really a Tilt error, not a user error.

[judofyr]

This seems to be caused by a bug in JRuby:

$LOAD_PATH << 'lib'
Object.autoload :Tilt, 'tilt'
p Object.autoload?(:Tilt)
require 'tilt'
p Object.autoload?(:Tilt)

JRuby incorrectly doesn't detect that the constant is no longer autoloaded:

$ ruby -v test.rb
ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-darwin14]
"tilt"
nil
$ ruby -v test.rb
jruby 9.0.4.0 (2.2.2) 2015-11-12 b9fb7aa Java HotSpot(TM) 64-Bit Server VM 24.45-b08 on 1.7.0_45-b18 +jit [darwin-x86_64]
"tilt"
"tilt"

[judofyr]

Added a test case to Tilt for this: https://travis-ci.org/rtomayko/tilt/builds/101022917

Open issue in JRuby: jruby/jruby#3585

[mojavelinux]

I think I know understand the problem better.

The fact is, requiring 'tilt/haml' doesn't actually change anything in Tilt. All it does is load the class. To restate, when you require 'tilt/haml', it doesn't register anything with the @template_map in Tilt.default_mapping. So it's still going to hit the lazy_load as soon as you try to access it via Tilt['haml'] (or equivalent).

All the warning message tells you is whether more than one thread is activate at the time the template is first looked up (no matter how explicitly it is done), which is sometimes true in JRuby and (Rubinius).

You can silence the message by explicitly registering the template.

require 'tilt/haml'
Tilt.register Tilt::HamlTemplate, 'haml'

This way, you can be sure that lazy_load is never run. I think at one point requiring tilt/haml did this (maybe) hence why the message suggests it. But it certainly doesn't do anything now.

[mojavelinux]

That whole script is now:

require 'tilt'
require 'tilt/haml'

# silence warning message about lazy loading tilt/haml
Tilt.register Tilt::HamlTemplate, 'haml'

template = Tilt['haml']
template.new 'test.html.haml', 1

[judofyr]

Yes, it will still hit lazy_load, but there's a part of lazy load that will first try to see if the constant already exists (without requiring anything). On JRuby this fails because it's saying that a constant is autoloadable, even though it's actually been loaded. I'm not quite sure what the workaround will be.

That said, I have considered also adding a mutex around the lazy loader and remove the message. If you're doing any thread-unsafe requiring you'll get a MutexError. This is also cached so there's no performance issues. On fre. 8. jan. 2016 at 20.44, Dan Allen notifications@github.com wrote:

That whole script is now:

require 'tilt'require 'tilt/haml'

silence warning message about lazy loading tilt/hamlTilt.register Tilt::HamlTemplate, 'haml'

template = Tilt['haml'] template.new 'test.html.haml', 1

—

Reply to this email directly or view it on GitHub https://github.com/asciidoctor/asciidoclet/issues/45#issuecomment-170104328 .

[mojavelinux]

Yes, it will still hit lazy_load, but there's a part of lazy load that will first try to see if the constant already exists (without requiring anything).

Oops, I missed that. Now I see what you're getting at. :+1:

[mojavelinux]

Thanks for your patience in working through this @judofyr!

[judofyr]

Thanks. Progress on Tilt is a bit slow, but I do like to see it survive and not rotten too much. Every now and then I try to get some proper work done on it :) On lør. 9. jan. 2016 at 00.55, Dan Allen notifications@github.com wrote:

Thanks for your patience in working through this @judofyr https://github.com/judofyr!

— Reply to this email directly or view it on GitHub https://github.com/asciidoctor/asciidoclet/issues/45#issuecomment-170159676 .

[judofyr]

Pushed out a workaround in Tilt for now: https://travis-ci.org/rtomayko/tilt/builds/101427393. Should be in the next patch version.

Going to add the mutex/monitor around the lazy loader (and remove the warning) in the next minor version.

[mojavelinux]

Fantastic news. Thanks @judofyr!

[jmuheim]

I'm getting a similar warning:

WARN: tilt autoloading 'tilt/pandoc' in a non thread-safe way; explicit require 'tilt/pandoc' suggested.

While `tilt/pandoc' isn't an official template yet, here's the PR: https://github.com/rtomayko/tilt/pull/276

Will this problem go away with a further release of tilt? Or do I have to do something? I already tried adding require 'tilt/pandoc' in application.rb, but didn't work out.

[mojavelinux]

Yes, a future version of Tilt will have a fix for this issue. Until then, you can safely ignore the message since it is actually isn't a warning at all.

[jmuheim]

Thanks.

[msgilligan]

I just submitted a PR to asciidoctorj to upgrade to the latest Tilt: https://github.com/asciidoctor/asciidoctorj/pull/656

@mojavelinux

[mojavelinux]

Seems reasonable to me.

[msgilligan]

I'm not sure we can close this issue -- it looks like the new Tilt breaks asciidoclet.

> Task :bitcoinj-json:javadoc FAILED
Jun 28, 2018 6:37:02 PM org.asciidoctor.internal.JRubyAsciidoctor render
SEVERE: org.jruby.RubyException
javadoc: error - In doclet class org.asciidoctor.Asciidoclet,  method start has thrown an exception java.lang.reflect.InvocationTargetException
org.asciidoctor.internal.AsciidoctorCoreException: org.jruby.exceptions.RaiseException: (TypeError) bind argument must be an instance of Tilt::CompiledTemplates
        at org.asciidoctor.internal.JRubyAsciidoctor.render(JRubyAsciidoctor.java:317)
        at org.asciidoctor.internal.JRubyAsciidoctor.render(JRubyAsciidoctor.java:424)
        at org.asciidoctor.asciidoclet.AsciidoctorRenderer.render(AsciidoctorRenderer.java:160)
        at org.asciidoctor.asciidoclet.AsciidoctorRenderer.renderDoc(AsciidoctorRenderer.java:108)
        at org.asciidoctor.asciidoclet.DocletIterator.renderClass(DocletIterator.java:71)
        at org.asciidoctor.asciidoclet.DocletIterator.render(DocletIterator.java:56)
        at org.asciidoctor.Asciidoclet.run(Asciidoclet.java:275)
        at org.asciidoctor.Asciidoclet.start(Asciidoclet.java:268)
        at org.asciidoctor.Asciidoclet.start(Asciidoclet.java:242)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
1 error
        at com.sun.tools.javadoc.DocletInvoker.invoke(DocletInvoker.java:310)
        at com.sun.tools.javadoc.DocletInvoker.start(DocletInvoker.java:189)
        at com.sun.tools.javadoc.Start.parseAndExecute(Start.java:366)
        at com.sun.tools.javadoc.Start.begin(Start.java:219)
        at com.sun.tools.javadoc.Start.begin(Start.java:205)
        at com.sun.tools.javadoc.Main.execute(Main.java:64)
        at com.sun.tools.javadoc.Main.main(Main.java:54)
Caused by: org.jruby.exceptions.RaiseException: (TypeError) bind argument must be an instance of Tilt::CompiledTemplates
        at org.jruby.RubyUnboundMethod.bind(org/jruby/RubyUnboundMethod.java:106)
        at RUBY.evaluate(/Users/sean/.gradle/caches/modules-2/files-2.1/org.asciidoctor/asciidoctorj/1.5.7/8e8c1d8fc6144405700dd8df3b177f2801ac5987/asciidoctorj-1.5.7.jar!/gems/tilt-2.0.8/lib/tilt/template.rb:170)
        at RUBY.evaluate(/Users/sean/.gradle/caches/modules-2/files-2.1/org.asciidoctor/asciidoctorj/1.5.7/8e8c1d8fc6144405700dd8df3b177f2801ac5987/asciidoctorj-1.5.7.jar!/gems/tilt-2.0.8/lib/tilt/haml.rb:46)
        at RUBY.render(/Users/sean/.gradle/caches/modules-2/files-2.1/org.asciidoctor/asciidoctorj/1.5.7/8e8c1d8fc6144405700dd8df3b177f2801ac5987/asciidoctorj-1.5.7.jar!/gems/tilt-2.0.8/lib/tilt/template.rb:109)
        at RUBY.convert(/Users/sean/.gradle/caches/modules-2/files-2.1/org.asciidoctor/asciidoctorj/1.5.7/8e8c1d8fc6144405700dd8df3b177f2801ac5987/asciidoctorj-1.5.7.jar!/gems/asciidoctor-1.5.7.1/lib/asciidoctor/converter/template.rb:196)
        at RUBY.convert(/Users/sean/.gradle/caches/modules-2/files-2.1/org.asciidoctor/asciidoctorj/1.5.7/8e8c1d8fc6144405700dd8df3b177f2801ac5987/asciidoctorj-1.5.7.jar!/gems/asciidoctor-1.5.7.1/lib/asciidoctor/converter/composite.rb:31)
        at RUBY.convert(/Users/sean/.gradle/caches/modules-2/files-2.1/org.asciidoctor/asciidoctorj/1.5.7/8e8c1d8fc6144405700dd8df3b177f2801ac5987/asciidoctorj-1.5.7.jar!/gems/asciidoctor-1.5.7.1/lib/asciidoctor/abstract_block.rb:71)
        at RUBY.content(/Users/sean/.gradle/caches/modules-2/files-2.1/org.asciidoctor/asciidoctorj/1.5.7/8e8c1d8fc6144405700dd8df3b177f2801ac5987/asciidoctorj-1.5.7.jar!/gems/asciidoctor-1.5.7.1/lib/asciidoctor/abstract_block.rb:80)
        at org.jruby.RubyArray.map(org/jruby/RubyArray.java:2414)
        at RUBY.content(/Users/sean/.gradle/caches/modules-2/files-2.1/org.asciidoctor/asciidoctorj/1.5.7/8e8c1d8fc6144405700dd8df3b177f2801ac5987/asciidoctorj-1.5.7.jar!/gems/asciidoctor-1.5.7.1/lib/asciidoctor/abstract_block.rb:80)
        at RUBY.content(/Users/sean/.gradle/caches/modules-2/files-2.1/org.asciidoctor/asciidoctorj/1.5.7/8e8c1d8fc6144405700dd8df3b177f2801ac5987/asciidoctorj-1.5.7.jar!/gems/asciidoctor-1.5.7.1/lib/asciidoctor/document.rb:1209)
        at RUBY.embedded(/Users/sean/.gradle/caches/modules-2/files-2.1/org.asciidoctor/asciidoctorj/1.5.7/8e8c1d8fc6144405700dd8df3b177f2801ac5987/asciidoctorj-1.5.7.jar!/gems/asciidoctor-1.5.7.1/lib/asciidoctor/converter/html5.rb:278)
        at RUBY.convert(/Users/sean/.gradle/caches/modules-2/files-2.1/org.asciidoctor/asciidoctorj/1.5.7/8e8c1d8fc6144405700dd8df3b177f2801ac5987/asciidoctorj-1.5.7.jar!/gems/asciidoctor-1.5.7.1/lib/asciidoctor/converter/base.rb:38)
        at RUBY.convert(/Users/sean/.gradle/caches/modules-2/files-2.1/org.asciidoctor/asciidoctorj/1.5.7/8e8c1d8fc6144405700dd8df3b177f2801ac5987/asciidoctorj-1.5.7.jar!/gems/asciidoctor-1.5.7.1/lib/asciidoctor/converter/composite.rb:31)
        at RUBY.convert(/Users/sean/.gradle/caches/modules-2/files-2.1/org.asciidoctor/asciidoctorj/1.5.7/8e8c1d8fc6144405700dd8df3b177f2801ac5987/asciidoctorj-1.5.7.jar!/gems/asciidoctor-1.5.7.1/lib/asciidoctor/document.rb:1141)
        at RUBY.convert(/Users/sean/.gradle/caches/modules-2/files-2.1/org.asciidoctor/asciidoctorj/1.5.7/8e8c1d8fc6144405700dd8df3b177f2801ac5987/asciidoctorj-1.5.7.jar!/gems/asciidoctor-1.5.7.1/lib/asciidoctor.rb:1515)
        at RUBY.convert(<script>:79)
        at org.jruby.gen.InterfaceImpl531616802.convert(org/jruby/gen/InterfaceImpl531616802.gen:13)

[judofyr]

@msgilligan: Do you have a reduced test case (with Gemfile/Gemfile.lock + used JRuby version) that I can test for?

[msgilligan]

No, but it is an open source project and I can put a failing branch on Travis CI.

[msgilligan]

The failing branch/PR is here: https://github.com/ConsensusJ/consensusj/pull/37 The failing Travis CI build is here: https://travis-ci.org/ConsensusJ/consensusj/builds/398315639 The commit with the breaking change is here: https://github.com/ConsensusJ/consensusj/commit/fd264ab2cdabae74ce06b4581fecc8a150664386 (Note: asciidoctorjVersion is 1.5.6 in gradle.properties and is overridden for Asciidoclet)

[judofyr]

I’ll try to look at it this weekend. If I don’t have time for it then, ping me again around July 9th and I should have more time then:))) On Fri, 29 Jun 2018 at 18:13 Sean Gilligan notifications@github.com wrote:

The failing branch/PR is here: ConsensusJ/consensusj#37 https://github.com/ConsensusJ/consensusj/pull/37 The failing Travis CI build is here: https://travis-ci.org/ConsensusJ/consensusj/builds/398315639 The commit with the breaking change is here: ConsensusJ/consensusj@fd264ab https://github.com/ConsensusJ/consensusj/commit/fd264ab2cdabae74ce06b4581fecc8a150664386 (Note: asciidoctorjVersion is 1.5.6 in gradle.properties and is overridden for Asciidoclet)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/asciidoctor/asciidoclet/issues/45#issuecomment-401402111, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAB84F-NjFy16_lbWsspgGzViF3b996ks5uBlIZgaJpZM4F5Jpa .

[judofyr]

I'm sorry, I don't know my way around Java/Gradle and I don't quite know how to debug this. According to ./gradlew dependencies it should use JRuby 9.1.16.0, but when I install that locally and run the Tilt tests everything seems to work fine. All the AsciiDoctor tests (for version 1.5.7.1) pass as well.

Do you know how I can modify the actual Ruby code being ran by ./gradlew buildCI? I see that it downloads .jar files that contains bundled Tilt, but I'm not quite sure how I can "patch" into that. If I'm able to insert some printing statements it would be much easier to debug.

[msgilligan]

@judofyr Thanks for looking at this. I'm an asciidoctor user and minor contributor. I don't understand the mechanisms used by AsciidoctorJ to invoke Ruby and Gems.

Let's see what @mojavelinux and @johncarl81 have to say.

It they don't have any ideas, I can try to make the simples possible Gradle script to reproduce the issue and possibly see if there's a way to reproduce it without Gradle at all.