Details

    • Type: Bug Bug
    • Status: Resolved Resolved
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: JRuby 1.7.1, JRuby 1.7.2
    • Fix Version/s: JRuby 1.7.3
    • Component/s: Embedding
    • Labels:
      None
    • Number of attachments :
      0

      Description

      After calling ScriptingContainer.clear() (which should release global variables and mappings), I'm getting a NullPointerException upon a runScriptlet() call:

      Caused by: java.lang.NullPointerException
      	at org.jruby.embed.internal.BiVariableMap.getLocalVarNames(BiVariableMap.java:338)
      	at org.jruby.embed.internal.EmbedRubyRuntimeAdapterImpl.getManyVarsDynamicScope(EmbedRubyRuntimeAdapterImpl.java:219)
      	at org.jruby.embed.internal.EmbedRubyRuntimeAdapterImpl.runParser(EmbedRubyRuntimeAdapterImpl.java:172)
      	at org.jruby.embed.internal.EmbedRubyRuntimeAdapterImpl.parse(EmbedRubyRuntimeAdapterImpl.java:92)
      	at org.jruby.embed.ScriptingContainer.parse(ScriptingContainer.java:1195)
      	at org.jruby.embed.ScriptingContainer.runScriptlet(ScriptingContainer.java:1255)
      	at 	... 35 more
      
      

        Activity

        Hide
        Hiro Asari added a comment -

        Do you have a reproducible test case?

        Show
        Hiro Asari added a comment - Do you have a reproducible test case?
        Hide
        Gergely Nagy added a comment -

        more info: this did not occur with Jruby 1.5.6 at least.
        Any workaround is welcome.
        Thanks!

        Show
        Gergely Nagy added a comment - more info: this did not occur with Jruby 1.5.6 at least. Any workaround is welcome. Thanks!
        Hide
        Gergely Nagy added a comment - - edited

        > Do you have a reproducible test case?
        Yes, I can do a pull request if you want but here it is:

        package org.jruby.embed;
        import static org.junit.Assert.*;
        
        import org.junit.Before;
        import org.junit.Test;
        
        public class EmbedClearBug7058Test {
            private ScriptingContainer c;
            @Before
            public void setUp() {
                c = new ScriptingContainer(LocalContextScope.THREADSAFE);
            }
            @Test
            public void testDoesNotThrowNPEAfterClear_Bug7058() {
                c.clear();
                c.runScriptlet(""); // throws NPE
            }
            @Test
            public void testClearReleasesGlobalVariable_EndlessLoopIn1_7_2() {
                c.runScriptlet("$a = 1");
                assertEquals(1L, c.getVarMap().get("$a"));
                c.clear();
                assertEquals(null, c.getVarMap().get("$a"));
            }
            @Test
            public void testClearReleasesGlobalVariable_NPEIn1_7_2() {
                c.runScriptlet("$a = 1");
                assertEquals(1L, c.runScriptlet("$a"));
                assertEquals(1L, c.getVarMap().get("$a"));
                c.clear();
                assertEquals(null, c.runScriptlet("$a"));
            }
            @Test
            public void testClearReleasesGlobalVariable_FailsIn1_6_7_NPEIn1_7_2() {
                c.runScriptlet("$a = 1");
                assertEquals(1L, c.runScriptlet("$a"));
                c.clear();
                assertEquals(null, c.runScriptlet("$a"));
            }
        
        }
        
        Show
        Gergely Nagy added a comment - - edited > Do you have a reproducible test case? Yes, I can do a pull request if you want but here it is: package org.jruby.embed; import static org.junit.Assert.*; import org.junit.Before; import org.junit.Test; public class EmbedClearBug7058Test { private ScriptingContainer c; @Before public void setUp() { c = new ScriptingContainer(LocalContextScope.THREADSAFE); } @Test public void testDoesNotThrowNPEAfterClear_Bug7058() { c.clear(); c.runScriptlet(""); // throws NPE } @Test public void testClearReleasesGlobalVariable_EndlessLoopIn1_7_2() { c.runScriptlet( "$a = 1" ); assertEquals(1L, c.getVarMap().get( "$a" )); c.clear(); assertEquals( null , c.getVarMap().get( "$a" )); } @Test public void testClearReleasesGlobalVariable_NPEIn1_7_2() { c.runScriptlet( "$a = 1" ); assertEquals(1L, c.runScriptlet( "$a" )); assertEquals(1L, c.getVarMap().get( "$a" )); c.clear(); assertEquals( null , c.runScriptlet( "$a" )); } @Test public void testClearReleasesGlobalVariable_FailsIn1_6_7_NPEIn1_7_2() { c.runScriptlet( "$a = 1" ); assertEquals(1L, c.runScriptlet( "$a" )); c.clear(); assertEquals( null , c.runScriptlet( "$a" )); } }
        Hide
        Gergely Nagy added a comment -

        I just need to ensure that global variables are reset/cleared between script runs.
        That exception is blocking my upgrade from 1.5.6 to 1.7.2 - so any easy workaround would be welcome.
        Thanks.

        Show
        Gergely Nagy added a comment - I just need to ensure that global variables are reset/cleared between script runs. That exception is blocking my upgrade from 1.5.6 to 1.7.2 - so any easy workaround would be welcome. Thanks.
        Hide
        Gergely Nagy added a comment -

        Added pull request for testcases https://github.com/jruby/jruby/pull/508

        Show
        Gergely Nagy added a comment - Added pull request for testcases https://github.com/jruby/jruby/pull/508
        Hide
        Mark Richards added a comment -

        I can verify that this is caused by a change to BiVariableMap.clear() after 1.7.0. The clear() method adds a null object to the variables member if you don't have a variable called "ARGV". After this happens, subsequent calls to jruby result in an endless loop later on in getVariable.

        I changed the clear() method as follows:

        line 472

        • variables.add(argv_object);
          + if (argv_object != null) variables.add(argv_object);

        and that fixed it for me.

        Show
        Mark Richards added a comment - I can verify that this is caused by a change to BiVariableMap.clear() after 1.7.0. The clear() method adds a null object to the variables member if you don't have a variable called "ARGV". After this happens, subsequent calls to jruby result in an endless loop later on in getVariable. I changed the clear() method as follows: line 472 variables.add(argv_object); + if (argv_object != null) variables.add(argv_object); and that fixed it for me.
        Hide
        Gergely Nagy added a comment -

        @Mark, yeah, thanks.
        Does this pass tests above?
        I was also looking at the code, and saw fishy things around there - but I would really let the maintainers to come up with a proper solution. But if you can commit or provide a pull request for them that would be great.
        This is still blocking my upgrade (and I just missed a window of opportunity there), I'm interested to speed this up. At least some attention would be nice. Thanks again!

        Show
        Gergely Nagy added a comment - @Mark, yeah, thanks. Does this pass tests above? I was also looking at the code, and saw fishy things around there - but I would really let the maintainers to come up with a proper solution. But if you can commit or provide a pull request for them that would be great. This is still blocking my upgrade (and I just missed a window of opportunity there), I'm interested to speed this up. At least some attention would be nice. Thanks again!
        Hide
        Yoko Harada added a comment -

        Hi,

        Sorry for taking so long to fix this bug. Finally, I pushed the fix whose revision is 41ae762 .

        Basically, @Mark is right. That add method added null without checking anything. That change fixed @Gergely's test cases.

        More than that, creating arrays while instantiating the BiVariableMap class was not a good idea. It turned out BiVariableMap had 10 nulls in arrays. I fixed as well.

        I also added @Gergely's test case to BiVariableMapTest so that it won't regress.

        Thanks for reporting this and patiently waiting the fix.

        Show
        Yoko Harada added a comment - Hi, Sorry for taking so long to fix this bug. Finally, I pushed the fix whose revision is 41ae762 . Basically, @Mark is right. That add method added null without checking anything. That change fixed @Gergely's test cases. More than that, creating arrays while instantiating the BiVariableMap class was not a good idea. It turned out BiVariableMap had 10 nulls in arrays. I fixed as well. I also added @Gergely's test case to BiVariableMapTest so that it won't regress. Thanks for reporting this and patiently waiting the fix.
        Hide
        Yoko Harada added a comment -

        The bug has been fixed.

        If there's further problem, please file a new issue.
        Or, reopening this issue would be an option if the bug is the same as this.

        Show
        Yoko Harada added a comment - The bug has been fixed. If there's further problem, please file a new issue. Or, reopening this issue would be an option if the bug is the same as this.

          People

          • Assignee:
            Yoko Harada
            Reporter:
            Gergely Nagy
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: