Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: JRuby 1.1.5
    • Fix Version/s: JRuby 1.6.6
    • Component/s: Interpreter
    • Labels:
      None
    • Environment:
      Any
    • Number of attachments :
      2

      Description

      In trunk JRuby, autoloads are not threadsafe. Specifically, multiple threads can simultaneously trigger an autoload, and mayhem results.

      1. 0001-Partial-work-for-JRUBY-3194-autoload-not-threadsafe.patch
        0.7 kB
        Charles Oliver Nutter
      2. jruby-3194.patch
        4 kB
        Charles Oliver Nutter

        Activity

        Hide
        Hiroshi Nakamura added a comment -

        Thank you! I could easily confirm the reproduction of the errors. I'll investigate it on master and write again. After all, I'll continue to backport the fixes to jruby-1_6.

        Show
        Hiroshi Nakamura added a comment - Thank you! I could easily confirm the reproduction of the errors. I'll investigate it on master and write again. After all, I'll continue to backport the fixes to jruby-1_6.
        Hide
        Hiroshi Nakamura added a comment -

        This should fix the issue on master.

        commit 698e1e39911f68536d7b670dddc0f5b90a0f3834
        Author: Hiroshi Nakamura <nahi@ruby-lang.org>
        Date:   Thu Jan 19 16:39:03 2012 +0900
        
            JRUBY-3194: Avoid NameError for concurrent autoload access
            
            Revert the synchronize change at d7ff5b9d.  I wrote 'removing
            unnecessary synchronized block' but I was wrong.
            
            IAutoloadMethod#load which is defined in RubyKernel.java removes
            Autoload object from RubyModule#autoloadMap after successful
            autoloading, and it must be done in synchronized block.  If not, other
            Threads could see orphan autoloadMap just between successful loading and
            autoloadMap clean-up by the initial Thread.  This causes NameError for
            constant access.
            
            PoC project is at [1].  Many thanks to David Kellum who prepared this
            for us!
            
            [1] https://github.com/dekellum/sinatra-threaded-autoload
        

        I'll try backporting to jruby-1_6.

        Show
        Hiroshi Nakamura added a comment - This should fix the issue on master. commit 698e1e39911f68536d7b670dddc0f5b90a0f3834 Author: Hiroshi Nakamura <nahi@ruby-lang.org> Date: Thu Jan 19 16:39:03 2012 +0900 JRUBY-3194: Avoid NameError for concurrent autoload access Revert the synchronize change at d7ff5b9d. I wrote 'removing unnecessary synchronized block' but I was wrong. IAutoloadMethod#load which is defined in RubyKernel.java removes Autoload object from RubyModule#autoloadMap after successful autoloading, and it must be done in synchronized block. If not, other Threads could see orphan autoloadMap just between successful loading and autoloadMap clean-up by the initial Thread. This causes NameError for constant access. PoC project is at [1]. Many thanks to David Kellum who prepared this for us! [1] https://github.com/dekellum/sinatra-threaded-autoload I'll try backporting to jruby-1_6.
        Hide
        Hiroshi Nakamura added a comment -

        Here's my brute-force backport to jruby-1_6: https://github.com/nahi/jruby/tree/autoload-threadsafe_16
        The diff from jruby-1_6: https://github.com/nahi/jruby/compare/jruby:jruby-1_6...nahi:autoload-threadsafe_16

        ant test passes. ant spec causes a few autoload spec failures caused by behavior change of autolaod.

        I have no idea how I can make this change opt-in...

        Show
        Hiroshi Nakamura added a comment - Here's my brute-force backport to jruby-1_6: https://github.com/nahi/jruby/tree/autoload-threadsafe_16 The diff from jruby-1_6: https://github.com/nahi/jruby/compare/jruby:jruby-1_6...nahi:autoload-threadsafe_16 ant test passes. ant spec causes a few autoload spec failures caused by behavior change of autolaod. I have no idea how I can make this change opt-in...
        Hide
        Charles Oliver Nutter added a comment -

        Reopening since there's some ongoing work here.

        Show
        Charles Oliver Nutter added a comment - Reopening since there's some ongoing work here.
        Hide
        Charles Oliver Nutter added a comment -

        We decided to just roll threadsafe autoloads directly into JRuby 1.6.6.

        Show
        Charles Oliver Nutter added a comment - We decided to just roll threadsafe autoloads directly into JRuby 1.6.6.

          People

          • Assignee:
            Charles Oliver Nutter
            Reporter:
            Yehuda Katz
          • Votes:
            2 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: