RxJava Android right use disposable

Question

I find right way use rxJava in android and see many samples like

public class MainActivity extends AppCompatActivity {

    CompositeDisposable compositeDisposable = new CompositeDisposable();

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        compositeDisposable.add(Single.fromCallable(
                () -> {
                    //some work here
                    return "result some work";
                })
                .subscribeOn(Schedulers.io())
                .observeOn(AndroidSchedulers.mainThread())
                .subscribe(r -> Log.e("MyLogs",r)));
    }

    @Override
    protected void onDestroy() {
        super.onDestroy();
        compositeDisposable.dispose();
    }
}

But is it correct? I mean is it a memory leak - save link to observer in compositeDisposable that not clear until activity not destroyed?

Answer 1

But is it correct? I mean is it a memory leak"

Your current code is not a memory leak and any observers are disposed of correctly.

However this is just the necessary RxJava ceremony required to ensure disposal of observers and that is not to say in another example it can't do something silly within the lambda and leak i.e. passing a reference to the Activity (enclosing scope) to another object which retains a strong reference to the enclosing scope, and which has a larger scope and no way to clear the reference.

To summerise this code doesn't leak but it can be more nuanced for more complex examples.

A CONTRIVED example showing the difference betweeen a memory leak and non-memory leak that demonstrates that even though the RxJava ceremony code is fine around disposing observers that is doesn't guarantee leak free code.

public class MainActivity extends AppCompatActivity {

    CompositeDisposable compositeDisposable = new CompositeDisposable();

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        compositeDisposable.add(Single.fromCallable(
                        () -> {
                            //some work here
                            return LeakySingleton.getInstance().doSomeWork(MainActivity.this);
                        })
                .subscribeOn(Schedulers.io())
                .observeOn(AndroidSchedulers.mainThread())
                .subscribe(r -> Log.e("MyLogs",r)));

        compositeDisposable.add(Single.fromCallable(
                        () -> {
                            //some work here
                            return NonLeakySingleton.getInstance().doSomeWork(MainActivity.this);
                        })
                .subscribeOn(Schedulers.io())
                .observeOn(AndroidSchedulers.mainThread())
                .subscribe(r -> Log.e("MyLogs",r)));
    }

    @Override
    protected void onDestroy() {
        super.onDestroy();
        compositeDisposable.dispose();
    }

    private static class LeakySingleton {

        // Caching map with Context as the key
        private final Map<Context, String> leakyMemoizationMap = new HashMap<>();

        private static class LeakySingletonHolder {
            private static final LeakySingleton INSTANCE = new LeakySingleton();
        }

        public static LeakySingleton getInstance() {
            return LeakySingletonHolder.INSTANCE;
        }

        public String doSomeWork(Context context) {
            // add context to the map - creating a strong reference that is never cleared, this is a memory leak
            return leakyMemoizationMap.computeIfAbsent(context, (ctx) -> "some calculated task that required a context");
        }
    }

    private static class NonLeakySingleton {

        // Caching map with Context as the key
        private final Map<Context, String> nonLeakyMemoizationMap = new WeakHashMap<>();

        private static class NonLeakySingletonHolder {
            private static final NonLeakySingleton INSTANCE = new NonLeakySingleton();
        }

        public static NonLeakySingleton getInstance() {
            return NonLeakySingletonHolder.INSTANCE;
        }

        public String doSomeWork(Context context) {
            // add context to the map - creating a weak reference, this is NOT a memory leak
            return nonLeakyMemoizationMap.computeIfAbsent(context, (ctx) -> "some calculated task that required a context");
        }
    }
}