|
I tend to agree with Vladimir that this should be an optional behavior. Because JRuby may be used in a server setting with lots of JRuby instances sharing a pool of threads, setting and using a thread context classloader could cause some pretty nasty bugs and conflicts. I would not be opposed to adding a flag or property that will cause JRuby to set it for you, if that would be useful. But it seems like setting it yourself might be the most reliable way. Thoughts? I'm not sure I fully understand the implications of using JRuby with thread pools in server environment, but I see that this issue of setting context class loader comes up very regularly: XMLDecoder, Spring container, active-record JDBC drivers, various other things (asked either on mailing list or on IRC). It would be great if we could resolve that somehow so that users won't need to struggle trying to figure out why it doesn't work. As for pooled threads, won't the resetting the context classloader when the thread goes back to pool work? Maybe, setting context classloader should be default, with a switch to turn it off (so that those who really see problems in server environment, would be able to disable that). Again, those who manage serious server deployments, should be pretty technical and skilled, so they should be able to figure that out and see classloader related issues quickly, while the general public that tries out JRuby with their favorite library might be better off with context loader set by default. The problem is knowing when the thread has gone back to the pool. We'd need to set it on the way in and the way out, but it's very difficult to know what is "the way in" given that you might call through other Java code, back into Ruby, back out into Java code, and so on. So in the case of a webapp in a thread-pooled server, the appropriate way to handle it would be for e.g. jruby-rack to set it on the way in and out, since jruby-rack knows when a request starts and ends. Adding that logic into JRuby core seems like it would just be asking for breakage. The option of having it on by default is certainly a possibility, but then again there's the possibility it would break simple apps that might use MVM. However I'm not opposed to having context classloader set by default, with an option to turn it off. Thread context classloader is mostly a hack in the JVM to get around the hierarchical nature of classloaders. But since libraries do use it, I suppose we need to consider a more JVM-friendly course of action by setting it always with an option to disable. I suppose we could do that on trunk and for 1.1.3 and see how it goes. Maybe it will work out fine... Marking for 1.1.3. We'll set context classloader for all JRuby threads by default and see how it goes. I don't think we should set it for Java threads that are "adopted" into the system. I haven't followed this thread closely but as someone deploying Rails apps on Java appservers frankly the idea of touching context classloader scares me. It may work for some appservers, but I'm not convinced it will work everywhere. If we can come up with an unobtrusive way to bootstrap JRuby with modified thread context classloaders, I'd be amenable to it but I don't want my existing apps to break. Would help if this stuff was more transparent to all of us. We probably need to build more test cases of places that need such support. I also don't understand why a java library would depend on thread context class loaders, since I thought that it is not set in some cases. There is an issue with the ThreadContext classloader and swingx as well. I wrote a very simple test with monkeybars to show the issue. After fighting with this for a day i was shown this ticket from Vladimir, (thnx.) I try to keep my code usable in NetBeans, the command line (jruby src/main.rb,) and in a jar, using rawr. It appears that with the JXTable, having cell's be editable works rather oddly with the classloader. I imagine this effects more than editing but thats just what i've noticed already. Also i will note that i do NOT require 'jarfile.jar', im adding the jarfile to $CLASSPATH Note: the table DOES show up without the patch, its just not editable. I found this extrmemely odd Howto get the code: git clone git://plastik.us/jruby_swingx_test.git Read the README file for using it (just two lines to comment/uncomment and jrake command) Ok, here's a proposal that might be amenable to all involved. Please comment.
So in short, we will only set TC classloader for threads we explicitly own, including the main thread when run from command line and threads started from Ruby. Thoughts? +1 on Charlie's latest comment. Implementation-wise, you'll only set it from org.jruby.Main? Nick: From org.jruby.Main, and in RubyNativeThread, which is launched and owned entirely by the JRuby runtime. The one problem I can think of that might be a little twitchy is if you're calling across JRuby instances, but we need to formalize that behind the MVM API anyway, which would not normally allow the same thread to cross VM boundaries. I think it's an acceptable risk for now. And to clarify, we would not set context classloader for any threads JRuby does not launch, which would include app server pooled threads. FYI, I think I'm going to need a test case for this to consider it resolved, even after I make the changes. So please, someone submit a simple test case that shows we're setting TC classloader appropriately for the main CLI thread and for sub threads. Do you have any existing tests that do anything like my test code. If so I could look into adapting my test if it apears to be the simplest way to create the test. I'm requiring a jar with a single class but then instantiating an instance of that class by passing an XML serialization of a class instance to XMLDecoder. This fails with a ClassNotFoundException unless the TC Classloader knows about the jar because by default XMLDecoder uses a TC Classloader. It wasn't clear to me how this fits in with how you are doing testing now. Here's a suggestion from a colleague (Scott Cytacki) for a form of test that tests the reverse – I think it's is a simplification of the app server environment. The general problem he is worried about is when you have 2 or more JRuby instances and they are sharing the same threads. So in that case you have to set the context class loader to one JRuby instance or another. Someone should try to make a test(s) that attempts to have 2 JRuby instances that use the same thread. Then it might be more clear what the issues are. Something like:
If the jruby instance sets the thread context class when it is invoked then the test above will probably fail. Here's a patch basically setting TC classloader in the places I described. This should cover the main thread as well as threads owned and launched by Ruby. Give this a go...I am a little strapped for time right now to test all these bugs, so we need some help. A couple issues/questions:
Clock's ticking on 1.1.3... Updated version that handles security exceptions gracefully. I verified that the updated patch at least fixes the problem with mysql-jdbc driver (when I use the driver installed as part of a gem instead of messing with CLASSPATH). Here's the example:
require 'rubygems'
require 'jdbc/mysql' # load jdbc dirver for mysql
require 'java'
# This not needed anymore, after the patch!
# require 'jruby'
# java.lang.Thread.currentThread.setContextClassLoader(JRuby.runtime.jruby_class_loader)
module Jdbc
DriverManager = java.sql.DriverManager
Driver = com.mysql.jdbc.Driver
end
url = "jdbc:mysql://fedora/base_development"
user = "user"
pass = "pass"
conn = Jdbc::DriverManager.get_connection(url, user, pass)
statement = conn.create_statement
query = "SELECT * FROM posts"
result_set = statement.execute_query(query)
# For each row returned do some stuff
while (result_set.next) do
post = Hash.new
post[:id] = result_set.getObject("id")
post[:title] = result_set.getObject("title")
puts post.inspect
end
statement.close
conn.close
This now works fine without any classpath magic (commented out in the sources above). Ok, I've committed this fix and the simplest possible test that just confirms the following:
Testing that external threads don't get a context classloader is not easily possible from within a Ruby script, since Java threads spawned by JRuby also naturally inherit JRuby's context classloader. So suggestions and improvements for the test are welcome. But we'll ship with this. |
||||||||||||||||||||||||||||||||||||||||||||||||
I've encountered another case when
proper thread context classloader is required: loading JDBC drivers
that are not on CLASSPATH.
Luckily, there is a more or less straightforward way to adjust the
context classloader:
With that, I verified that the test case from the bug report pass. And
JDBC drivers are loaded OK too.