Scala Programming Language
  1. Scala Programming Language
  2. SI-6969

race condition in structural type polycache

    Details

      Description

      I guess I never opened this ticket. My bad.

      Please see http://www.scala-lang.org/node/11807 for further elaboration.

      I got this NPE just now:
      [partest] java.lang.NullPointerException
      [partest] at
      scala.reflect.internal.SymbolTable$perRunCaches$$anonfun$clearAll$2.reflMethod$Method3(SymbolTable.scala:240)
      [partest] at
      scala.reflect.internal.SymbolTable$perRunCaches$$anonfun$clearAll$2.apply(SymbolTable.scala:240)
      [partest] at
      scala.reflect.internal.SymbolTable$perRunCaches$$anonfun$clearAll$2.apply(SymbolTable.scala:235)
      [partest] at scala.collection.mutable.HashSet.foreach(HashSet.scala:75)
      [partest] at
      scala.reflect.internal.SymbolTable$perRunCaches$.clearAll(SymbolTable.scala:235)
      

      ...

      (from Pavel Pavlov)
      
      There is incorrect use of soft references in synthetic method:
      =============================================
      public static Method reflMethod$Method1(Class class1) {
          if((MethodCache)reflPoly$Cache1.get() == null)
              reflPoly$Cache1 = new SoftReference(new EmptyMethodCache());
          Method method1 = ((MethodCache)reflPoly$Cache1.get()).find(class1);
          if(method1 == null) {
              method1 = ScalaRunTime$.MODULE$.ensureAccessible(class1.getMethod("clear", reflParams$Cache1));
              reflPoly$Cache1 = new SoftReference(((MethodCache)reflPoly$Cache1.get()).add(class1, method1));
          }
          return method1;
      }
      =============================================
      
      Between any of the three calls to reflPoly$Cache1.get() soft reference can be cleared by GC.
      
      The correct code so should be:
      
      =============================================
      public static Method reflMethod$Method1(Class class1) {
          MethodCache cache = (MethodCache)reflPoly$Cache1.get();
          if(cache == null) {
              cache = new EmptyMethodCache();
          }
          Method method1 = cache.find(class1);
          if(method1 == null) {
              method1 = ScalaRunTime$.MODULE$.ensureAccessible(class1.getMethod("clear", reflParams$Cache1));
              reflPoly$Cache1 = new SoftReference(cache.add(class1, method1));
          }
          return method1;
      }
      =============================================
      

        Activity

        Show
        Paul Phillips added a comment - https://github.com/scala/scala/pull/1902
        Hide
        Simon Ochsenreither added a comment -

        Merged in https://github.com/scala/scala/commit/417304514b664e5f66bfc06fb2b0e86b99d23a63 for 2.10.1.

        Avian and probably JamVM were affected by this issue.

        (Adding this so it can be found when searching for “Avian”.)

        Shouldn't this fix be ported to 2.9.3 and 2.11.0?

        Show
        Simon Ochsenreither added a comment - Merged in https://github.com/scala/scala/commit/417304514b664e5f66bfc06fb2b0e86b99d23a63 for 2.10.1. Avian and probably JamVM were affected by this issue. (Adding this so it can be found when searching for “Avian”.) Shouldn't this fix be ported to 2.9.3 and 2.11.0?
        Hide
        Paul Phillips added a comment -

        Patches to 2.10.x still reach 2.11 automatically.

        Backports to 2.9.x are not performed as a matter of course. Contributions welcome.

        Show
        Paul Phillips added a comment - Patches to 2.10.x still reach 2.11 automatically. Backports to 2.9.x are not performed as a matter of course. Contributions welcome.
        Hide
        Simon Ochsenreither added a comment - - edited

        2.9.x: https://github.com/scala/scala/pull/1972

        I guess now the people in charge need to decide whether it is important enough.

        Show
        Simon Ochsenreither added a comment - - edited 2.9.x: https://github.com/scala/scala/pull/1972 I guess now the people in charge need to decide whether it is important enough.
        Hide
        Adriaan Moors added a comment -

        I apologize for missing this. To make it easier for me to discover your fixes, please set the fix version field to the version you believe this should be fixed. Similarly for milestones on github. We're working on automation for the latter.

        Show
        Adriaan Moors added a comment - I apologize for missing this. To make it easier for me to discover your fixes, please set the fix version field to the version you believe this should be fixed. Similarly for milestones on github. We're working on automation for the latter.
        Hide
        Simon Ochsenreither added a comment -

        No problem, my mistake!

        But in general, I'm always unsure which version should be mentioned. Is there a guide to the Scala issue tracker somewhere?
        E. g. is the fix field for “has been fixed in” or “should be fixed in”? If it is the second option, than what does “affects version”? Or does the meaning switch from one to another depending on the Open/Closed status?

        Show
        Simon Ochsenreither added a comment - No problem, my mistake! But in general, I'm always unsure which version should be mentioned. Is there a guide to the Scala issue tracker somewhere? E. g. is the fix field for “has been fixed in” or “should be fixed in”? If it is the second option, than what does “affects version”? Or does the meaning switch from one to another depending on the Open/Closed status?
        Hide
        Adriaan Moors added a comment -

        it's a sad state of affairs we need a guide to this

        "fix version" is the version when we expect to fix the issue
        "affects version" indicates where the problem appeared

        Show
        Adriaan Moors added a comment - it's a sad state of affairs we need a guide to this "fix version" is the version when we expect to fix the issue "affects version" indicates where the problem appeared
        Hide
        Simon Ochsenreither added a comment -

        I'm terribly sorry. :-/
        I guess I'll have to put a post-it note on my display, otherwise I have forgotten it the next time again . I'm getting old...

        Show
        Simon Ochsenreither added a comment - I'm terribly sorry. :-/ I guess I'll have to put a post-it note on my display, otherwise I have forgotten it the next time again . I'm getting old...
        Hide
        Adriaan Moors added a comment -

        Sorry, by sad state of affairs I meant our affairs/JIRA, not yours
        I wish JIRA's UI design would make these things self-apparent.

        Please start a thread on scala-internals if things are still unclear.

        Show
        Adriaan Moors added a comment - Sorry, by sad state of affairs I meant our affairs/JIRA, not yours I wish JIRA's UI design would make these things self-apparent. Please start a thread on scala-internals if things are still unclear.

          People

          • Assignee:
            Paul Phillips
            Reporter:
            Paul Phillips
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development