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

Commit 0ec832bb authored by Soonil Nagarkar's avatar Soonil Nagarkar
Browse files

Only allow completion callback to be run once for PIs 

PendingIntent.send() should either throw an exception OR run it's
completion callback, but not both. However, a bug that is not fixable in
the near term means that it's possible for both to happen. In order to
avoid the completion callback from being run even if send() fails, we
gate it.

Bug: 199464864
Test: manual + presubmits
Change-Id: I993436b2bd2ec5240be92e6d3815b3a9edef970f
parent ea07bec3

services/core/java/com/android/server/location/provider/LocationProviderManager.java

+56 −2
Original line number Diff line number Diff line
@@ -243,15 +243,24 @@ public class LocationProviderManager extends 
                intent.putExtra(KEY_LOCATIONS, locationResult.asList().toArray(new Location[0]));

            }



            // send() SHOULD only run the completion callback if it completes successfully. however,

            // b/199464864 (which could not be fixed in the S timeframe) means that it's possible

            // for send() to throw an exception AND run the completion callback. if this happens, we

            // would over-release the wakelock... we take matters into our own hands to ensure that

            // the completion callback can only be run if send() completes successfully. this means

            // the completion callback may be run inline - but as we've never specified what thread

            // the callback is run on, this is fine.

            GatedCallback gatedCallback = new GatedCallback(onCompleteCallback);



            mPendingIntent.send(

                    mContext,

                    0,

                    intent,

                    onCompleteCallback != null ? (pI, i, rC, rD, rE) -> onCompleteCallback.run()

                            : null,

                    (pI, i, rC, rD, rE) -> gatedCallback.run(),

                    null,

                    null,

                    options.toBundle());

            gatedCallback.allow();

        }



        @Override
@@ -2734,4 +2743,49 @@ public class LocationProviderManager extends 
            }

        }

    }



    private static class GatedCallback implements Runnable {



        private @Nullable Runnable mCallback;



        @GuardedBy("this")

        private boolean mGate;

        @GuardedBy("this")

        private boolean mRun;



        GatedCallback(Runnable callback) {

            mCallback = callback;

        }



        public void allow() {

            Runnable callback = null;

            synchronized (this) {

                mGate = true;

                if (mRun && mCallback != null) {

                    callback = mCallback;

                    mCallback = null;

                }

            }



            if (callback != null) {

                callback.run();

            }

        }



        @Override

        public void run() {

            Runnable callback = null;

            synchronized (this) {

                mRun = true;

                if (mGate && mCallback != null) {

                    callback = mCallback;

                    mCallback = null;

                }

            }



            if (callback != null) {

                callback.run();

            }

        }

    }

}