Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

race condition in structural type polycache #6969

Closed
scabug opened this issue Jan 14, 2013 · 10 comments
Closed

race condition in structural type polycache #6969

scabug opened this issue Jan 14, 2013 · 10 comments
Assignees
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Jan 14, 2013

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;
}
=============================================
@scabug
Copy link
Author

@scabug scabug commented Jan 14, 2013

Imported From: https://issues.scala-lang.org/browse/SI-6969?orig=1
Reporter: @paulp
Affected Versions: 2.10.0

@scabug
Copy link
Author

@scabug scabug commented Jan 15, 2013

@scabug
Copy link
Author

@scabug scabug commented Jan 25, 2013

@soc said:
Merged in scala/scala@4173045 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?

@scabug scabug closed this Jan 25, 2013
@scabug
Copy link
Author

@scabug scabug commented Jan 25, 2013

@paulp said:
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.

@scabug
Copy link
Author

@scabug scabug commented Jan 25, 2013

@soc said (edited by @adriaanm on Feb 1, 2013 7:10:21 PM UTC):
2.9.x: scala/scala#1972

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

@scabug
Copy link
Author

@scabug scabug commented Feb 1, 2013

@adriaanm said:
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.

@scabug
Copy link
Author

@scabug scabug commented Feb 1, 2013

@soc said:
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?

@scabug
Copy link
Author

@scabug scabug commented Feb 2, 2013

@adriaanm said:
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

@scabug
Copy link
Author

@scabug scabug commented Feb 3, 2013

@soc said:
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...

@scabug
Copy link
Author

@scabug scabug commented Feb 21, 2013

@adriaanm said:
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.

@scabug scabug added this to the 2.10.1-RC1 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants