Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 451d9c9a authored by Abhijeet Kaur's avatar Abhijeet Kaur
Browse files

Add synchronized locks to bugreport callback methods 

BugreportCallbackImpl class methods modify (read/write) the private
variable mInfo. Add synchronized locks for consistency.

Make other methods static to be thread-safe, and operate only on its
arguments.

Bug: 123617758
Test: Build and flash. Takes bugreport as expected
Change-Id: I237fd940f5ccd4833d053558cf1b014883d59791
parent 712d0f7d

packages/Shell/src/com/android/shell/BugreportProgressService.java

+88 −67
Original line number Diff line number Diff line
@@ -381,6 +381,7 @@ public class BugreportProgressService extends Service { 


    private final class BugreportCallbackImpl extends BugreportCallback {



        @GuardedBy("mLock")

        private final BugreportInfo mInfo;



        BugreportCallbackImpl(BugreportInfo info) {
@@ -389,10 +390,12 @@ public class BugreportProgressService extends Service { 


        @Override

        public void onProgress(float progress) {

            synchronized (mLock) {

                if (progress == 0) {

                trackInfoWithId();

                    trackInfoWithIdLocked();

                }

                checkProgressUpdatedLocked(mInfo, (int) progress);

            }

            checkProgressUpdated(mInfo, (int) progress);

        }



        /**
@@ -401,18 +404,21 @@ public class BugreportProgressService extends Service { 
         */

        @Override

        public void onError(@BugreportErrorCode int errorCode) {

            trackInfoWithId();

            stopProgress(mInfo.id);

            synchronized (mLock) {

                trackInfoWithIdLocked();

                stopProgressLocked(mInfo.id);

            }

            Log.e(TAG, "Bugreport API callback onError() errorCode = " + errorCode);

            return;

        }



        @Override

        public void onFinished() {

            // TODO: Make all callback functions lock protected.

            mInfo.renameBugreportFile();

            mInfo.renameScreenshots(mScreenshotsDir);

            sendBugreportFinishedBroadcast();

            synchronized (mLock) {

                sendBugreportFinishedBroadcastLocked();

            }

        }



        /**
@@ -421,7 +427,8 @@ public class BugreportProgressService extends Service { 
         * when dumpstate calls one of the callback functions (onProgress, onFinished, onError)

         * after the id has been incremented.

         */

        private void trackInfoWithId() {

        @GuardedBy("mLock")

        private void trackInfoWithIdLocked() {

            final int id = SystemProperties.getInt(PROPERTY_LAST_ID, 1);

            if (mBugreportInfos.get(id) == null) {

                mInfo.id = id;
@@ -430,53 +437,55 @@ public class BugreportProgressService extends Service { 
            return;

        }



        private void sendBugreportFinishedBroadcast() {

        @GuardedBy("mLock")

        private void sendBugreportFinishedBroadcastLocked() {

            final String bugreportFilePath = mInfo.bugreportFile.getAbsolutePath();

            if (mInfo.bugreportFile.length() == 0) {

                Log.e(TAG, "Bugreport file empty. File path = " + bugreportFilePath);

                return;

            }

            if (mInfo.type == BugreportParams.BUGREPORT_MODE_REMOTE) {

                sendRemoteBugreportFinishedBroadcast(bugreportFilePath, mInfo.bugreportFile);

                sendRemoteBugreportFinishedBroadcast(mContext, bugreportFilePath,

                        mInfo.bugreportFile);

            } else {

                trackInfoWithId();

                trackInfoWithIdLocked();

                cleanupOldFiles(MIN_KEEP_COUNT, MIN_KEEP_AGE);

                final Intent intent = new Intent(INTENT_BUGREPORT_FINISHED);

                intent.putExtra(EXTRA_BUGREPORT, bugreportFilePath);

                addScreenshotToIntent(intent);

                addScreenshotToIntent(intent, mInfo);

                mContext.sendBroadcast(intent, android.Manifest.permission.DUMP);

                onBugreportFinished(mInfo.id);

            }

        }

    }



        private void sendRemoteBugreportFinishedBroadcast(String bugreportFileName,

                File bugreportFile) {

    private static void sendRemoteBugreportFinishedBroadcast(Context context,

            String bugreportFileName, File bugreportFile) {

        cleanupOldFiles(REMOTE_BUGREPORT_FILES_AMOUNT, REMOTE_MIN_KEEP_AGE);

        final Intent intent = new Intent(DevicePolicyManager.ACTION_REMOTE_BUGREPORT_DISPATCH);

            final Uri bugreportUri = getUri(mContext, bugreportFile);

        final Uri bugreportUri = getUri(context, bugreportFile);

        final String bugreportHash = generateFileHash(bugreportFileName);

        if (bugreportHash == null) {

            Log.e(TAG, "Error generating file hash for remote bugreport");

                return;

        }

        intent.setDataAndType(bugreportUri, BUGREPORT_MIMETYPE);

        intent.putExtra(DevicePolicyManager.EXTRA_REMOTE_BUGREPORT_HASH, bugreportHash);

        intent.putExtra(EXTRA_BUGREPORT, bugreportFileName);

            mContext.sendBroadcastAsUser(intent, UserHandle.SYSTEM,

        context.sendBroadcastAsUser(intent, UserHandle.SYSTEM,

                android.Manifest.permission.DUMP);

    }



        private void addScreenshotToIntent(Intent intent) {

            final File screenshotFile = mInfo.screenshotFiles.isEmpty()

                    ? null : mInfo.screenshotFiles.get(0);

            if (screenshotFile != null && screenshotFile.length() > 0) {

    private static void addScreenshotToIntent(Intent intent, BugreportInfo info) {

        final String screenshotFileName = info.name + ".png";

        final File screenshotFile = new File(BUGREPORT_DIR, screenshotFileName);

        final String screenshotFilePath = screenshotFile.getAbsolutePath();

        if (screenshotFile.length() > 0) {

            intent.putExtra(EXTRA_SCREENSHOT, screenshotFilePath);

        }

        return;

    }



        private String generateFileHash(String fileName) {

    private static String generateFileHash(String fileName) {

        String fileHash = null;

        try {

            MessageDigest md = MessageDigest.getInstance("SHA-256");
@@ -498,7 +507,6 @@ public class BugreportProgressService extends Service { 
        }

        return fileHash;

    }

    }



    static void cleanupOldFiles(final int minCount, final long minAge) {

        new AsyncTask<Void, Void, Void>() {
@@ -852,7 +860,8 @@ public class BugreportProgressService extends Service { 
    /**

     * Finalizes the progress on a given bugreport and cancel its notification.

     */

    private void stopProgress(int id) {

    @GuardedBy("mLock")

    private void stopProgressLocked(int id) {

        if (mBugreportInfos.indexOfKey(id) < 0) {

            Log.w(TAG, "ID not watched: " + id);

        } else {
@@ -883,7 +892,9 @@ public class BugreportProgressService extends Service { 
            }

            deleteScreenshots(info);

        }

        stopProgress(id);

        synchronized (mLock) {

            stopProgressLocked(id);

        }

    }



    /**
@@ -1178,7 +1189,9 @@ public class BugreportProgressService extends Service { 
        if (!info.bugreportFile.exists() || !info.bugreportFile.canRead()) {

            Log.e(TAG, "Could not read bugreport file " + info.bugreportFile);

            Toast.makeText(context, R.string.bugreport_unreadable_text, Toast.LENGTH_LONG).show();

            stopProgress(info.id);

            synchronized (mLock) {

                stopProgressLocked(info.id);

            }

            return;

        }
@@ -1290,7 +1303,9 @@ public class BugreportProgressService extends Service { 
        final Intent sendIntent = buildSendIntent(mContext, info);

        if (sendIntent == null) {

            Log.w(TAG, "Stopping progres on ID " + id + " because share intent could not be built");

            stopProgress(id);

            synchronized (mLock) {

                stopProgressLocked(id);

            }

            return;

        }
@@ -1313,9 +1328,10 @@ public class BugreportProgressService extends Service { 
        } else {

            mContext.startActivity(notifIntent);

        }



        synchronized (mLock) {

            // ... and stop watching this process.

        stopProgress(id);

            stopProgressLocked(id);

        }

    }



    static void sendShareIntent(Context context, Intent intent) {
@@ -2361,14 +2377,18 @@ public class BugreportProgressService extends Service { 
                // The right, long-term solution is to provide an onFinished() callback

                // on IDumpstateListener and call it instead of using a broadcast.

                Log.w(TAG, "Dumpstate process died:\n" + info);

                stopProgress(info.id);

                synchronized (mLock) {

                    stopProgressLocked(info.id);

                }

            }

            token.asBinder().unlinkToDeath(this, 0);

        }



        @Override

        public void onProgress(int progress) throws RemoteException {

            checkProgressUpdated(info, progress);

            synchronized (mLock) {

                checkProgressUpdatedLocked(info, progress);

            }

        }



        @Override
@@ -2387,7 +2407,8 @@ public class BugreportProgressService extends Service { 


    }



    private void checkProgressUpdated(BugreportInfo info, int progress) {

    @GuardedBy("mLock")

    private void checkProgressUpdatedLocked(BugreportInfo info, int progress) {

        if (progress > CAPPED_PROGRESS) {

            progress = CAPPED_PROGRESS;

        }