Android runOnUiThread crash when pressing Back button

Question

I am using the DownloadManager to download files from a server, I'm doing it from an adapter, and I'm showing the download progress in the fragment using a runOnUiThread. it's working great, but when I press the back button while downloading, the runOnUiThread throws a NullPointerException. I tried putting a variable in the onPause with an if statement to stop the method but it doesn't work either.

what do you recommend to do to solve this crash?

here's my code:

new Thread(new Runnable() {
                        @Override
                        public void run() {

                            downloading = true;

                            while (downloading) {

                                DownloadManager.Query q = new DownloadManager.Query();
                                q.setFilterById(myDownloadReference);

                                Cursor cursor = downloadManager.query(q);
                                cursor.moveToFirst();
                                int bytes_downloaded = cursor.getInt(cursor
                                        .getColumnIndex(DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR));
                                int bytes_total = cursor.getInt(cursor.getColumnIndex(DownloadManager.COLUMN_TOTAL_SIZE_BYTES));

                                if (cursor.getInt(cursor.getColumnIndex(DownloadManager.COLUMN_STATUS)) == DownloadManager.STATUS_SUCCESSFUL) {
                                    downloading = false;
                                    downloadingDocs.remove(doc);

                                    //remove the progressBar when download is complete
                                    fragment.getActivity().runOnUiThread(new Runnable() {
                                        @Override
                                        public void run() {
                                            finalHolder2.downloadProgress.setVisibility(View.INVISIBLE);
                                        }
                                    });

                                    //check if it's the last download and open it.
                                    if (downloadingDocs.size() == 0) {
                                        actionIntentOpenDocs(doc, saveFile);
                                    }
                                }
                                //final double dl_progress = (bytes_downloaded / bytes_total) * 100;
                                final int dl_progress = (int) ((bytes_downloaded * 100l) / bytes_total);

                                //display the download progress
                                fragment.getActivity().runOnUiThread(new Runnable() {
                                    @Override
                                    public void run() {
                                        if(Constants.stopUiThreadWork == null) {
                                            finalHolder2.downloadProgress.setProgress((int) dl_progress);
                                        }
                                    }
                                });
                                cursor.close();
                            }
                        }
                    }).start();

it's crashing on this line :

fragment.getActivity().runOnUiThread(new Runnable() {

Crash stack trace :

FATAL EXCEPTION: Thread-4807
Process: com.appus.app, PID: 6553
java.lang.NullPointerException: Attempt to invoke virtual method 'void android.support.v4.app.FragmentActivity.runOnUiThread(java.lang.Runnable)' on a null object reference
        at com.availo.app.UI.adapters.MainFragmentDocumentAdapter$1$1.run(MainFragmentDocumentAdapter.java:265)
        at java.lang.Thread.run(Thread.java:818)

Answer 1

try to check for your activity before you execute runOnUiThread like this:

    /** check if activity still exist */
    if (getActivity() == null) {
        return;
    }

    getActivity().runOnUiThread(new Runnable() {
    ...
    });

"you shouldn't have returns littered through your code? (...)"
"cleaner" is where it gets ambiguous. We're not writing assembler, so I don't see how that's relevant here. Apples and oranges. When writing methods, having a single return where possible improves readability and maintenance. If your code is getting complex or requires multiple return statements by following this coding style, then it's likely that the method requires further analysis and breakdown. – giulio

@giulio - references, publications, specs - i don't see any in your comment - but i can give you some to read and gain some knowledge.

You can start from here to blow your mind:

• https://softwareengineering.stackexchange.com/questions/118703/where-did-the-notion-of-one-return-only-come-from

and then read my favorite:

http://www.yegor256.com/2015/08/18/multiple-return-statements-in-oop.html

Answer 2

The best way to manage this kind of thing is through a Service. The reason is that a service can better manage background threads, even if the activity isn't running, while things like an AsyncTask will stop working when the activity is done. Take a look at this site on how to set up a Download Service.