Chainfire/libsuperuser

Bug: addCommand causes IllegalAccessError

AndroidDeveloperLB opened this issue · 10 comments

Suppose I have this initialized:

private var rootSession: Shell.Interactive? = ...
And now I call this:

        rootSession!!.addCommand(commands, 0, object : Shell.OnCommandResultListener {
            override fun onCommandResult(commandCode: Int, exitCode: Int, output: MutableList<String>?) {
                   ...

I get this exception:

Caused by: java.lang.IllegalAccessError: Illegal class access: ... attempting to access 'eu.chainfire.libsuperuser.Shell$OnResult'

Same occurs when using OnCommandResultListener2 .

How come?
I think it's because OnResult is marked as private.

Interesting, this seems to be a Kotlin-specific issue.

Can you please try on both Kotlin and Java?

I'm not going to (re-)learn Kotlin and create a sample app to close this issue, no. I'll change OnResult to public, and you can let me know if it works.

OK.
Can you please at least tell me if it works fine on Java?

It does work fine in Java.

Apparently Kotlin explicitly casts your (for example) Shell.OnCommandResultListener to Shell.OnResult when you call addCommand (Java does not do this). The bug report has been open for years with many people annoyed by it, but no mention of a fix.

Making OnResult public should fix the problem.

Can you please tell me how to test it out?
I mean, what to put exactly as dependency to check it out that it works on Kotlin?

Should auto-update, no? But specifically:

dependencies {
    implementation 'eu.chainfire:libsuperuser:1.1.0.201907261845'
}

Seems fine now.
Why was it needed to switch from OnCommandResultListener to OnCommandResultListener2 ?
Will OnCommandResultListener still work fine?

In which cases should we look at the error stream? Can you show an example of when it's useful?

OnCommandListener2 separates out STDERR from STDOUT. You can still use OnCommandListener if you prefer, Shell.Builder::wantSTDERR() then determines if you get STDERR output as well or just STDOUT.

A lot of shell commands write successful output to STDOUT and error output to STDERR. Separating them saves you from having to decide what is an error and what is normal output by parsing. Of course exit code also often tells you if there was an error or not. It all depends on what commands you're using, really.

So using OnCommandListener is perfectly fine, right?
Why deprecate it though?
Couldn't it be possible to use a single list, that has a String and a type for it (normal and error) ?
I think it can be more convenient...
The splitting of 2 streams mean we are out of sync in times...