Imported From: https://issues.scala-lang.org/browse/SI-6594?orig=1
Reporter: François-Xavier Thomas (frx)
Affected Versions: 2.10.0-RC1, 2.10.0-RC2
Attachments:

• Build.scala (created on Nov 13, 2012 7:47:13 AM UTC, 1285 bytes)
• MainActivity.scala (created on Nov 13, 2012 7:47:13 AM UTC, 531 bytes)
• unsafe.patch (created on Oct 30, 2012 6:14:03 PM UTC, 3243 bytes)

@viktorklang said:
I've spoken to Doug Lea about this, the sneakyThrow-esque approach should work.

@viktorklang said:
IMHO this is a blocker, good news is that my patch should solve it.

François-Xavier Thomas (frx) said:
Recompiling it right now, I suppose all I needed to do was apply patch and ant build?

François-Xavier Thomas (frx) said:
Seems like your patch made the issue disappear (at least for now), but Proguard is complaining about missing class definitions in the scala library, and Dalvik refuses to start the app. I might have to configure Proguard a little bit more. Will keep you posted.

François-Xavier Thomas (frx) said:
Uh, okay. Must have been something else. I was compiling the wrong version (2.11) of the Scala libraries.
Anyway, now I've got the 2.10-RC1-Patch compiled. I wanted to make sure, and decompiling the ForkJoinTask class present in the scala-library.jar yields the following, so your patch is indeed present :

> javap -c ForkJoinTask | grep throw                                                                                                         2.10.0-RC1-Patch [adaa5d2] untracked
   27:	athrow
   135:	invokestatic	#48; //Method rethrow:(Ljava/lang/Throwable;)V
   202:	invokestatic	#48; //Method rethrow:(Ljava/lang/Throwable;)V
public final java.lang.Object get()   throws java.lang.InterruptedException, java.util.concurrent.ExecutionException;
   39:	athrow
   63:	athrow
public final java.lang.Object get(long, java.util.concurrent.TimeUnit)   throws java.lang.InterruptedException, java.util.concurrent.ExecutionException, java.util.concurrent.TimeoutException;
   13:	athrow
   244:	athrow
   314:	athrow
   327:	athrow
   355:	athrow
   370:	athrow
   390:	athrow
   56:	ldc	#116; //String throwException
   100:	athrow

It seems fine...
...and I get the exact same error running it on Dalvik. Bummer. Seems that sun.misc.Unsafe doesn't have throwException.

I'm not a huge Java hacker, so I do not know the specifics, but what's the reason behind the use of sun.misc.Unsafe.throwException instead of the normal throw keyword?

François-Xavier Thomas (frx) said:
Other thread talking about the same thing : https://groups.google.com/forum/#!msg/scala-debate/El3QZEbIwHg/j-v35izchYQJ

François-Xavier Thomas (frx) said:
And a StackOverflow thread with a rather hacky technique to achieve the same thing as Unsafe (I think) : http://stackoverflow.com/questions/4375828/throwing-an-exception-using-unsafe-getunsafe-throwexception

Will try to adapt it to the ForkJoinTask later to see if it works, even though I'm kinda shooting in the dark here.

@viktorklang said:
If you remove the Unsafely-class from my patch and only use Softly.rethrow, what happens?

We can't use "throw" because Java doesn't allow you to throw anything but RuntimeException without a throws clause, and we cannot add that.
If you have read my patch you see that the Softly.rethrow is using the same technique described in the SO post.

@phaller said:
Viktor, do you have time to take a look at Doug's suggestion to use {{sneakyThrow}}? If not, I can take care of it. Just let me know if you'd like to take this issue or not.

@viktorklang said:
Actually it was I who suggested the sneakyThrow to Doug ;-) (it is used in my patch here, but only as a fallback when throwException doesn't exist, but is seems like Android is a bit more eager with verification than HotSpot is)
I just want to know first if the fix actually fixes it on Android, and I have no such environment to test it in so François-Xavier needs to confirm first.

@phaller said:
Ah, OK. Cool!
OK, then let's first see if your patch fixes it.

@viktorklang said:
François-Xavier, please report back ASAP on how it fares without the Unsafely-class, we need to cut the next RC very soon and I'd love to be able to ship that with Android working.

François-Xavier Thomas (frx) said:
I'm trying it out right now, I need to figure out what Maven is doing first. Android apparently complains as soon as you even try to define the throwException method in your code, so I'm going to try removing Unsafely like you suggested. Will tell you once it's finished compiling.

François-Xavier Thomas (frx) said:
Okay. Really sorry for being an idiot, but it turns out that, contrary to what I thought, the exception was totally unrelated to Unsafe missing, even though they were right next to each other. The real issue was at ForkJoinPool.java:2845 when trying to introspect a class member. Which, of course, thanks to Proguard, didn't work at all.

Adding -dontobfuscate to the Proguard configuration solves the issue.

@viktorklang said:
So what do you mean? This isn't any problem at all (i.e. ticket == invalid) or that my initial patch fixes the problem?

François-Xavier Thomas (frx) said (edited on Oct 31, 2012 7:45:47 PM UTC):
The former. Obfuscation really messed up everything. We can close the ticket now, I think.

Anyway, there should be a way to declare the the whole Scala libs as non-obfuscable by default in the Proguard configuration. I'm going to look into that, and maybe report my findings in the wiki, since there's very little official documentation. (https://www.assembla.com/wiki/show/scala-ide/Developing_for_Android ?)

@viktorklang said:
Alright, if it works as is I'll close the ticket now.

@viktorklang said:
Problem was due to the reportees use of ProGuard, not a bug.

Martin Kneissl (mkneissl) said:
I can reproduce this bug and it is not related to obfuscation / ProGuard.

Martin Kneissl (mkneissl) said:
Attached sources reproducing the problem with scala 2.10.0-RC2 and Android 4.0.4 (on Samsung Galaxy SII, but this should not matter at all).

Martin Kneissl (mkneissl) said:
The error messages are

I/dalvikvm(16110): Could not find method sun.misc.Unsafe.throwException, referenced from method scala.concurrent.forkjoin.ForkJoinPool.deregisterWorker
W/dalvikvm(16110): VFY: unable to resolve virtual method 13088: Lsun/misc/Unsafe;.throwException (Ljava/lang/Throwable;)V
D/dalvikvm(16110): VFY: replacing opcode 0x6e at 0x0084
I/dalvikvm(16110): Could not find method sun.misc.Unsafe.throwException, referenced from method scala.concurrent.forkjoin.ForkJoinTask.reportException
W/dalvikvm(16110): VFY: unable to resolve virtual method 13088: Lsun/misc/Unsafe;.throwException (Ljava/lang/Throwable;)V
D/dalvikvm(16110): VFY: replacing opcode 0x6e at 0x000d

This is caused by Dalvik in Android 4 not having sun.misc.Unsafe.throwExeption.
The main problem is not that the message appears in the log (the class still loads), but that the call to Unsafe.throwException will not happen if the corresponding code is executed.
I'll try the attached patch, soon.

@viktorklang said:
So why did it start working for the other guy?

You'll need to confirm whether this is a bug or not ASAP if the fix is to be able to be included in Scala 2.10

François-Xavier Thomas (frx) said:
I also still get those warnings from Dalvik, but AFAIK exceptions are handled correctly, using the non-patched version, and I've had no issue whatsoever using futures with that.
But bear in mind that I really am a beginner in both Java and Scala, so I might have missed something.

@viktorklang said:
You didn't mention that you still had the warnings, which definitely look menacing to me.

@martin: Does the patch fix it? If not, if you remove the "Unsafely" and only use "Softly" does that fix the situation?

François-Xavier Thomas (frx) said:
@viktor: After some trial and error, exceptions are reported, except in the rare case where an exception occurs in the onFailure method. I was just happy it worked, and I didn't consider this case since it never happened to me. Okay, I agree this is definitely not a good thing for the release, so I'll try the patch later tonight, will keep you posted.

@viktorklang said:
What's even worse is that I told Doug Lea that it wasn't a problem, as he offered to change to sneakyThrows in jsr166y.

The boat might have sailed on that now.

Martin Kneissl (mkneissl) said:
Well, it's quite simple: Dalvik's sun.misc.Unsafe doesn't have the methods. What do you expect when you call them? So its clearly a bug.

Here is the Unsafe class: http://omapzoom.org/?p=platform/libcore.git;a=history;f=luni/src/main/java/sun/misc/Unsafe.java;h=33bb9f1a0660acbb9e75ab0d252f55ea91c1cfd0;hb=HEAD

As promised I'll try the patched version, but I haven't compiled Scala recently, so this might take a few hours...

Martin Kneissl (mkneissl) said (edited on Nov 13, 2012 9:06:06 PM UTC):
The patch removes 2 of the 4 occurrences of the Info/Warning on missing Unsafe.throwException. (That's clear, because ForkJoinPool still uses it.)

Now, with the patched library, calling ForkJoinTask.rethrow() (via reflection / setAccessible) throws the passed exception.

Before, with the unpatched library, we get NoSuchMethodError.

So, please patch ForkJoinPool in the same way and this issue should be solved.

(BTW, building scala works like a charm!)
[edit for the unpatched case, corrected and rerun my test case]

François-Xavier Thomas (frx) said:
@martin: Thanks for the link ;)

Anyway, I just finished applying the patch as well, and came to the same conclusions. I only had 2 warnings though, with one remaining after the patch.

@adriaanm said:
I don't see any reason why this can't go into 2.10.1. Sorry.

an RC is only open to:

• serious regressions w.r.t milestones or RCs of the current major release.
• blocker bugs (i.e., the final would be utterly unusable in a typical project)
• critical bugs that cannot be fixed in the next minor release due to binary incompatibility.

@viktorklang said (edited on Nov 14, 2012 12:37:12 AM UTC):
It's ultimately something between the Scala-Android users and you Adriaan; I trust your judgement.

In any case, I have a fix, and it's practically risk-free.

I cannot comment on the impact of not fixing it as I have no experience with Dalvik.

@adriaanm said:
I don't like to think of this as being about me. It's about our policy.

Your patch doesn't change the public API, right? That means it can go into 2.10.1.
There's also no pressing reason (as defined by the policy I summarized above) for it going into 2.10.0.

Seems like an open-and-shut case to me.

@viktorklang said:
Sorry about the confusion, I meant "you" in the "Scala maintainers" sense.

As I said, I trust your judgement, I cannot say if it's critical for Dalvik or not,
and it could be a regression from early 2.10 milestones, but all I can do is to provide a fix.

@adriaanm said:
Thanks. Please submit a pull request for inclusion into the branch 2.10.x, that way it'll end up 2.10.1.

If there is no workaround and this makes Scala unusable on Android, I would be open to considering this a blocker.
However, Android is not an official platform for us, and requires custom tooling anyway,
so I imagine this can be worked around until 2.10.1.

If not, someone please speak up in the next couple of days.

François-Xavier Thomas (frx) said (edited on Nov 14, 2012 11:42:39 AM UTC):
Honestly, while I agree this is a bad thing to leave things unpatched, I've never run into any issue with the non-patched RC1 aside from what's discussed above and my own misuse of Proguard. And it's certainly not a pressing issue for me, what I'm doing is more or less a test project to become more familiar with both Scala and Android. So I'd be fine with it being included in 2.10.1. But that's just me, maybe others disagree!

Martin Kneissl (mkneissl) said:
Even though I use Scala on Android for real projects, I'm fine with the fix being included in 2.10.1 assuming it will be out within a few months after 2.10.0. Until then I can use the patch in an inofficial branch.

@viktorklang said:
I'm fixing this for Akka 2.0.4 which will be released against Scala 2.9 and has it's own FJP, Scala can use the same scheme to fix its FJP: https://github.com/akka/akka/pull/866/files

@adriaanm said:
Thanks for the feedback!

2.10.1-RC1 is scheduled for Feb 11.

@viktorklang said:
Don't use the aforementioned Akka 2.0.4 fix as we found it to be flawed, use: scala/scala#1838