Skip to content

Commit

Permalink
Merge pull request #396 from bitmold/leaky_async_tasks
Browse files Browse the repository at this point in the history 
Guards Against Context Memory Leaking
  • Loading branch information
@n8fr8
n8fr8 committed Oct 7, 2020
2 parents 3d4ddd9 + 7a09756 commit 1cbcd4f
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 98 deletions.
93 changes: 44 additions & 49 deletions app/src/main/java/org/torproject/android/OrbotMainActivity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@

import java.io.File;
import java.io.UnsupportedEncodingException;
import java.lang.ref.WeakReference;
import java.net.URLDecoder;
import java.net.URLEncoder;
import java.text.NumberFormat;
Expand Down Expand Up @@ -130,47 +131,8 @@ public class OrbotMainActivity extends AppCompatActivity implements OrbotConstan
private SharedPreferences mPrefs = null;
private boolean autoStartFromIntent = false;
// this is what takes messages or values from the callback threads or other non-mainUI threads
//and passes them back into the main UI thread for display to the user
private Handler mStatusUpdateHandler = new Handler() {

@Override
public void handleMessage(final Message msg) {


Bundle data = msg.getData();

switch (msg.what) {
case MESSAGE_TRAFFIC_COUNT:

DataCount datacount = new DataCount(data.getLong("upload"), data.getLong("download"));

long totalRead = data.getLong("readTotal");
long totalWrite = data.getLong("writeTotal");

downloadText.setText(String.format("%s / %s", formatCount(datacount.Download), formatTotal(totalRead)));
uploadText.setText(String.format("%s / %s", formatCount(datacount.Upload), formatTotal(totalWrite)));

break;
case MESSAGE_PORTS:

int socksPort = data.getInt("socks");
int httpPort = data.getInt("http");

lblPorts.setText(String.format(Locale.getDefault(), "SOCKS: %d | HTTP: %d", socksPort, httpPort));

break;
default:
String newTorStatus = msg.getData().getString("status");
String log = (String) msg.obj;

if (torStatus == null && newTorStatus != null) //first time status
findViewById(R.id.frameMain).setVisibility(View.VISIBLE);
updateStatus(log, newTorStatus);
super.handleMessage(msg);
break;
}
}
};
// and passes them back into the main UI thread for display to the user
private Handler mStatusUpdateHandler = new MainActivityStatusUpdateHandler(this);
/**
* The state and log info from {@link OrbotService} are sent to the UI here in
* the form of a local broadcast. Regular broadcasts can be sent by any app,
Expand Down Expand Up @@ -1233,15 +1195,48 @@ private void exportTorData() {

}

public static class DataCount {
// data uploaded
final long Upload;
// data downloaded
final long Download;
private static class MainActivityStatusUpdateHandler extends Handler {
private WeakReference<OrbotMainActivity> ref;

MainActivityStatusUpdateHandler(OrbotMainActivity oma) {
ref = new WeakReference<>(oma);
}

private boolean shouldStop() {
return ref.get() == null || ref.get().isFinishing();
}

@Override
public void handleMessage(final Message msg) {
if (shouldStop()) return;
OrbotMainActivity oma = ref.get();
Bundle data = msg.getData();

switch (msg.what) {
case MESSAGE_TRAFFIC_COUNT:
long upload = data.getLong("upload");
long download = data.getLong("download");
long totalRead = data.getLong("readTotal");
long totalWrite = data.getLong("writeTotal");
oma.downloadText.setText(String.format("%s / %s", oma.formatCount(download), oma.formatTotal(totalRead)));
oma.uploadText.setText(String.format("%s / %s", oma.formatCount(upload), oma.formatTotal(totalWrite)));
break;

case MESSAGE_PORTS:
int socksPort = data.getInt("socks");
int httpPort = data.getInt("http");
oma.lblPorts.setText(String.format(Locale.getDefault(), "SOCKS: %d | HTTP: %d", socksPort, httpPort));
break;

default:
String newTorStatus = msg.getData().getString("status");
String log = (String) msg.obj;

DataCount(long Upload, long Download) {
this.Upload = Upload;
this.Download = Download;
if (oma.torStatus == null && newTorStatus != null) //first time status
oma.findViewById(R.id.frameMain).setVisibility(View.VISIBLE);
oma.updateStatus(log, newTorStatus);
super.handleMessage(msg);
}
}
}
}
63 changes: 41 additions & 22 deletions app/src/main/java/org/torproject/android/ui/AppManagerActivity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.torproject.android.service.util.Prefs;
import org.torproject.android.service.vpn.TorifiedApp;

import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -105,29 +106,13 @@ public boolean onOptionsItemSelected(MenuItem item) {
}

private void reloadApps() {
new AsyncTask<Void, Void, Void>() {
protected void onPreExecute() {
// Pre Code
progressBar.setVisibility(View.VISIBLE);
}

protected Void doInBackground(Void... unused) {
loadApps(mPrefs);
return null;
}

protected void onPostExecute(Void unused) {
listApps.setAdapter(adapterApps);
progressBar.setVisibility(View.GONE);
}
}.execute();


new ReloadAppsAsyncTask(this).execute();
}

private void loadApps(SharedPreferences prefs) {
private void loadApps() {

if (mApps == null)
mApps = getApps(prefs);
mApps = getApps(mPrefs);

Collections.sort(mApps, (o1, o2) -> {
/* Some apps start with lowercase letters and without the sorting being case
Expand Down Expand Up @@ -279,7 +264,6 @@ public ArrayList<TorifiedApp> getApps(SharedPreferences prefs) {
return apps;
}


public void saveAppSettings() {

StringBuilder tordApps = new StringBuilder();
Expand All @@ -300,7 +284,6 @@ public void saveAppSettings() {
setResult(RESULT_OK, response);
}


public void onClick(View v) {

CheckBox cbox = null;
Expand All @@ -321,6 +304,42 @@ else if (v.getTag() instanceof CheckBox)
}
}

private static class ReloadAppsAsyncTask extends AsyncTask<Void, Void, Void> {

private WeakReference<AppManagerActivity> activity;

ReloadAppsAsyncTask(AppManagerActivity activity) {
this.activity = new WeakReference<>(activity);
}

@Override
protected void onPreExecute() {
if (shouldStop()) return;
activity.get().progressBar.setVisibility(View.VISIBLE);
}

@Override
protected Void doInBackground(Void... voids) {
if (shouldStop()) return null;
activity.get().loadApps();
return null;
}

@Override
protected void onPostExecute(Void unused) {
if (shouldStop()) return;
AppManagerActivity ama = activity.get();
ama.listApps.setAdapter(ama.adapterApps);
ama.progressBar.setVisibility(View.GONE);
}

private boolean shouldStop() {
AppManagerActivity ama = activity.get();
return ama == null || ama.isFinishing();
}

}

private static class ListEntry {
private CheckBox box;
private TextView text;
Expand Down
43 changes: 27 additions & 16 deletions app/src/main/java/org/torproject/android/ui/onboarding/BridgeWizardActivity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.lang.ref.WeakReference;
import java.net.InetSocketAddress;
import java.net.Socket;
import java.net.SocketAddress;
Expand All @@ -34,7 +35,7 @@ public class BridgeWizardActivity extends AppCompatActivity {
private static final int CUSTOM_BRIDGES_REQUEST_CODE = 1312;
private static final String BUNDLE_KEY_TV_STATUS_VISIBILITY = "visibility";
private static final String BUNDLE_KEY_TV_STATUS_TEXT = "text";
private static TextView mTvStatus;
private TextView mTvStatus;
private static HostTester runningHostTest;
private RadioButton mBtDirect;
private RadioButton mBtObfs4;
Expand Down Expand Up @@ -182,7 +183,7 @@ protected void onActivityResult(int requestCode, int resultCode, @Nullable Inten

private void testBridgeConnection() {
cancelHostTestIfRunning();
HostTester hostTester = new HostTester();
HostTester hostTester = new HostTester(this);
if (TextUtils.isEmpty(Prefs.getBridgesList()) || (!Prefs.bridgesEnabled())) {
hostTester.execute("check.torproject.org", "443");
} else if (Prefs.getBridgesList().equals("meek")) {
Expand Down Expand Up @@ -253,43 +254,53 @@ private void evaluateBridgeListState() {
}
}

private class HostTester extends AsyncTask<String, Void, Boolean> {
private static class HostTester extends AsyncTask<String, Void, Boolean> {

private WeakReference<BridgeWizardActivity> ref;

HostTester(BridgeWizardActivity activity) {
ref = new WeakReference<>(activity);
}

private boolean shouldStop() {
return ref.get() == null || ref.get().isFinishing();
}

@Override
protected void onPreExecute() {

if (mTvStatus != null) {
// Pre Code
mTvStatus.setVisibility(View.VISIBLE);
mTvStatus.setText(mBtDirect.isChecked() ? R.string.testing_tor_direct : R.string.testing_bridges);
if (shouldStop()) return;
BridgeWizardActivity bwa = ref.get();
if (bwa.mTvStatus != null) {
bwa.mTvStatus.setVisibility(View.VISIBLE);
bwa.mTvStatus.setText(bwa.mBtDirect.isChecked() ? R.string.testing_tor_direct : R.string.testing_bridges);
}
}

@Override
protected Boolean doInBackground(String... host) {
// Background Code
for (int i = 0; i < host.length; i++) {
if (isCancelled()) return null;
if (shouldStop() || isCancelled()) return null;
String testHost = host[i];
i++; //move to the port
int testPort = Integer.parseInt(host[i]);
if (isHostReachable(testHost, testPort, 5000)) {
return true;
}
}

return false;
}

@Override
protected void onPostExecute(Boolean result) {
// Post Code
if (mTvStatus != null) {
if (shouldStop()) return;
BridgeWizardActivity bwa = ref.get();
if (bwa.mTvStatus != null) {
runningHostTest = null;
if (result) {
int stringRes = mBtDirect.isChecked() ? R.string.testing_tor_direct_success : R.string.testing_bridges_success;
mTvStatus.setText(stringRes);
int stringRes = bwa.mBtDirect.isChecked() ? R.string.testing_tor_direct_success : R.string.testing_bridges_success;
bwa.mTvStatus.setText(stringRes);
} else {
mTvStatus.setText(R.string.testing_bridges_fail);
bwa.mTvStatus.setText(R.string.testing_bridges_fail);
}
}
}
Expand Down
18 changes: 7 additions & 11 deletions orbotservice/src/main/java/org/torproject/android/service/vpn/Tun2Socks.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ public class Tun2Socks {
private static final String TAG = Tun2Socks.class.getSimpleName();
private static final boolean LOGD = true;

private static Thread mThread;
private static ParcelFileDescriptor mVpnInterfaceFileDescriptor;
private static int mVpnInterfaceMTU;
private static String mVpnIpAddress;
Expand All @@ -49,7 +48,6 @@ public class Tun2Socks {
private static String mUdpgwServerAddress;
private static boolean mUdpgwTransparentDNS;
private static HashMap<Integer, String> mAppUidBlacklist = new HashMap<>();
private static Context mContext;

static {
System.loadLibrary("tun2socks");
Expand All @@ -70,7 +68,6 @@ public static void Start(
String socksServerAddress,
String udpgwServerAddress,
boolean udpgwTransparentDNS) {
mContext = context;

mVpnInterfaceFileDescriptor = vpnInterfaceFileDescriptor;
mVpnInterfaceMTU = vpnInterfaceMTU;
Expand Down Expand Up @@ -120,17 +117,16 @@ private native static int runTun2Socks(

private native static void terminateTun2Socks();

public static boolean checkIsAllowed(int protocol, String sourceAddr, int sourcePort, String destAddr, int destPort) {

public static boolean checkIsAllowed(Context context, int protocol, String sourceAddr, int sourcePort, String destAddr, int destPort) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
return isAllowedQ(protocol, sourceAddr, sourcePort, destAddr, destPort);
return isAllowedQ(context, protocol, sourceAddr, sourcePort, destAddr, destPort);
} else
return isAllowed(protocol, sourceAddr, sourcePort, destAddr, destPort);
return isAllowed(context, protocol, sourceAddr, sourcePort, destAddr, destPort);
}

public static boolean isAllowed(int protocol, String sourceAddr, int sourcePort, String destAddr, int destPort) {
public static boolean isAllowed(Context context, int protocol, String sourceAddr, int sourcePort, String destAddr, int destPort) {

TCPSourceApp.AppDescriptor aInfo = TCPSourceApp.getApplicationInfo(mContext, sourceAddr, sourcePort, destAddr, destPort);
TCPSourceApp.AppDescriptor aInfo = TCPSourceApp.getApplicationInfo(context, sourceAddr, sourcePort, destAddr, destPort);

if (aInfo != null) {
int uid = aInfo.getUid();
Expand All @@ -140,8 +136,8 @@ public static boolean isAllowed(int protocol, String sourceAddr, int sourcePort,
}

@TargetApi(Build.VERSION_CODES.Q)
public static boolean isAllowedQ(int protocol, String sourceAddr, int sourcePort, String destAddr, int destPort) {
ConnectivityManager cm = (ConnectivityManager) mContext.getSystemService(CONNECTIVITY_SERVICE);
public static boolean isAllowedQ(Context context, int protocol, String sourceAddr, int sourcePort, String destAddr, int destPort) {
ConnectivityManager cm = (ConnectivityManager) context.getSystemService(CONNECTIVITY_SERVICE);
if (cm == null)
return false;

Expand Down

0 comments on commit 1cbcd4f

Please sign in to comment.