su request in main thread? i dont do much android, but on linux i always piped it through to a worker or even bash... yuck

Yay, I am not a developer, but Yays all the way

I say yay, let it be done!

examples first we need to learn :-P

There's reasons to call su from the main-thread: waiting for su success or failure. It's like a root-prompt on console, or Admin UI on Windows. It's the proper way for a UI to work: waiting for a prompt response. However, after su success everything following should be in an idling background thread.

The impression I'm getting is everybody does this wrong (not keeping the pipe open), doesn't wait nor properly get the su success response back, and doesn't bg-thread the rest.

I have Java/NDK code for this if you want to discuss.

The real problem is there isn't a proper setuid() interface to make these calls properly like UNIX / *BSD / OSX / Linux. Then we can bg-thread the entirety and stop this "SuperUser" app hack.

Or even the "Vista" proxy model where a worker process always has SU access, and then supports RPC requests from validated/PGP/ssh/etc apps with the proper key.

yeah, never make a main thread wait for something, unless you are making a pure terminal based application.

I started by exec()ing on the main thread, but I quickly learned while testing why that is such a bad idea.  Instead, I create an AsyncTask which handles the details of calling Runtime.getRuntime().exec("su <args>") on a background thread and synchronizing with the main thread to update the UI.

I'd love to see your sample code to see if I am Doing It RIght, or if there are some corner cases and race conditions I didn't think of.

A strict mode would be fine by me as a developer and a user.  I can imagine some users of poorly-written root apps would be annoyed; so perhaps a per-app override.

Speaking as a user, not a developer, I prefer the transition period as short as possible! I would suggest 1 month of warnings through Toast or Notification (with the option to disable this), but default setting "allow", then 1 month of warning and default setting "deny", then denying without Toast/Notification, but appearing in the Log only.

Rather than "naming and shaming" in public, how about an effort (perhaps with some trustworthy helpers) to notify as many of the affected developers as possible? Give them some time to resolve their issues? Perhaps a month to resolve?

I'd imagine at some point there would be a "naming and shaming", unfortunately that is just inevitable. Maybe at this point a list could be maintained and kudos given (application removed with a note) once the developer adopts this "better/safer/correct" method.

Just an idea.

Joseph, sorry, but you're wrong here. I do understand what you mean, but this is just not the way Android works or is designed. The main thread does not wait for a prompt response. Not when using "su", not when creating a popup yourself. We just don't do blocking calls on the main thread of which you have no idea how long they will take. Proper UI on other systems or not, this is simply not how we do things on Android. It's a surefire way of getting an ANR freeze.

Jason, similar example code will be provided. However, you should start a "su" shell and pipe commands into it, rather than calling "su" with commands as parameters, due to a number of bugs regarding parameter passing in the exec call, which can create havoc. Unfortunately, a great many devs still do call it from the main thread :( This is also the reason why the popup countdown is 10 seconds, not a minute, because a minute would cause an ANR in the calling app.

Brad, I agree, there may be no way to avoid naming and shaming, if only to reduce the amount of support questions, because user's will be able to look up if an app is a problematic or not.

+Chainfire - Thank you.

I was considering moving the commands out of the su parameters and pipe them instead because of a difference between how SuperSU does logging and access requests (by APK) and how Superuser does it (by APK and commandline).

Now that you have confirmed that piping the commands is the right thing to do, I'll go ahead and make the change.

Plz fix comp superSU with arabic

I'm a competent coder, just not a dabbler in coding Android apps. In that case, I'm merely an end user. What you're proposing is using a sledge to kill a mosquito. Here I am, happy-go-lucky (say) Titanium user (not saying this has coding problems, just picking it as an example of one app which needs to use su), and in order to remain current/secure/up-to-date, I "upgrade" to a newer SuperSU, and suddenly I'm broken, and in order to get fixed, I'm @ the mercy of (for example) the Ti author to un-break things. It's either that, or become a FOSS fanatic, & run ONLY FOSS programs on my tablet, &...oh, by the way...spend probably weeks or maybe months learning Android, and go fix it myself.

So "nay," unless it's an option (reminiscent of the "trust only Play for app installations" toggle)

How is this worse than for example when Android changed to not allow network communication in the main thread ? Did you speak out against that ?

Also, this will not suddenly break things - these things are already broken, and already causing crashes. The difference is that now it will be clear where the crash came from, and coders will know what they have to do to fix it - instead of it just appearing as a random problem without a cause.

Another big difference is that when we change this, instead of end-users complaining to me or Superuser's author about an app not working, they'll complain to whomever wrote the app that is broken in the first place.

That's why we will have a transitional period - to give authors a chance to update and fix before the shit hits the fan. And even then, it is likely it will be an option you can disable for a while longer.

It's only using a sledge to kill a mosquito if you prefer random crashes over fluent operation.

here is my su static function that logs error if on the main thread

public static String su(String cmd) {

if(Looper.myLooper() == Looper.getMainLooper()){

Log.e("SHELL","SU Running on main UI thread!!! "+cmd);

}

try {

Process process = Runtime.getRuntime().exec("su");

DataOutputStream writer=new DataOutputStream(process.getOutputStream());

BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()));

writer.writeBytes(cmd+"\n");

writer.writeBytes("exit\n");

writer.flush();

writer.close();

int read;

char[] buffer = new char[4096];

StringBuffer output = new StringBuffer();

while ((read = reader.read(buffer)) > 0) {

output.append(buffer, 0, read);

}

reader.close();

process.waitFor();

return output.toString();

} catch(Exception e){

Log.e("shell", "SU Command error: "+e.getMessage());

e.printStackTrace();

return "";

}

}

Uncanny how similar that is to my own code. Anyway, we will continue this when I return from holidays :)

I guess that's good then :) have fun on your trip

So I can't mention +Chainfire because sharing is disabled? Wtf G+?

Anyways, I think we're saying the same thing.

I'd JNI to a native-lib that does low-level "shell-work" since it's more natural there than Java. Lib tries to su and hold the su state (like a Vista proxy process and setuid). These JNI callouts are proxied by an internal Java Thread that handles it as IPC messages from the main-thread like the event-loop in Windows/Mobile

So you're right, I don't do it from the main-thread, but the worker thread does wait for the SU prompt to complete, returns su() success/failure status, and stuff I don't see su() apps doing.

It would be so much nicer if the SuperUser app would send out a notification of su() success so shell-trickery isn't needed.  SuperSU does that?

Yes please!

Shitty code is just shit!

Great initiative +Chainfire

Granted there's a large subset of root users who will have no idea what's wrong with their root apps, but you could implement it as a function of the supersu app, have a toggle to automatically deny root access to apps that incorrectly call for it then provide an error message every time an app is denied. This way if the end user can live with a poorly coded root app they can still allow it to function, but by default you can just have it deny all apps that are using it improperly

I for once think there needs to be a possiblity to enable backwards-compatibility. 

So put an option in settings including an additional Info on the problem and deactivate it by default (or wait a few weeks...)

Breaking legacy-apps or even having to install an outdated superuser app is a no-no in my eyes!

Actually, in the article that I wrote about it I show ways and code you can use in your project that will tell you when you are calling su from the main thread.

I thought it would be possible for SuperSU itself to produce an error when called from a program's main thread, but that actually does not appear to be possible, so all talk about that is moot anyway :)

I understand at the moment requesting superuser is simply an exec call to su?

You could aswell define a new interface and stay in the android-world: add a remote service that does all the nasty stuff for you, make it threaded intern, return values asynchronously and there you go: simple to use (except from jni of course :D) and no possiblity of mis-usage.

//Clearification: The remote service would be the terminal emulator, running su as soon as its first called.

Every direct call to su could then be called outdated and the user can be notified...

Only thing that needs a bit of though is how to get the su only to take commands from the superSU app without notifying the user. But every app is signed anyway so its feasable.

Also it would cause some kind of "fragmentation" (read: people with other su apps (read: miui) would need to download your app or miui et al needs to adopt your interface.

On the plus side with every command going through your app it's possible to introduce a mild security layer (or is it already?) (Ask the user before rm -rf / :D)