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

Commit 70b600d4 authored by Andreas Gampe's avatar Andreas Gampe
Browse files

Frameworks: Move SharedPreferencesImpl to Future

The asynchronous loading code is not safe wrt/ exceptions. Instead
of adding a tri-state for loading, move the code to use a Future
for the map. This encapsulates the required wait & synchronization,
as well as propagating any exceptions.

Bug: 67986472
Test: m
Test: Device boots
Test: m cts && cts-tradefed run commandAndExit cts-dev --module CtsContentTestCases -c android.content.cts.SharedPreferencesTest
Change-Id: I6616e8a05e64eb1cfe024cc3239a05847dfe1fab
parent f78a5175
Loading
Loading
Loading
Loading
+74 −54
Original line number Diff line number Diff line
@@ -50,6 +50,11 @@ import java.util.Map;
import java.util.Set;
import java.util.WeakHashMap;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.FutureTask;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;

final class SharedPreferencesImpl implements SharedPreferences {
    private static final String TAG = "SharedPreferencesImpl";
@@ -69,15 +74,11 @@ final class SharedPreferencesImpl implements SharedPreferences {
    private final Object mLock = new Object();
    private final Object mWritingToDiskLock = new Object();

    @GuardedBy("mLock")
    private Map<String, Object> mMap;
    private Future<Map<String, Object>> mMap;

    @GuardedBy("mLock")
    private int mDiskWritesInFlight = 0;

    @GuardedBy("mLock")
    private boolean mLoaded = false;

    @GuardedBy("mLock")
    private StructTimespec mStatTimestamp;

@@ -105,27 +106,18 @@ final class SharedPreferencesImpl implements SharedPreferences {
        mFile = file;
        mBackupFile = makeBackupFile(file);
        mMode = mode;
        mLoaded = false;
        mMap = null;
        startLoadFromDisk();
    }

    private void startLoadFromDisk() {
        synchronized (mLock) {
            mLoaded = false;
        }
        new Thread("SharedPreferencesImpl-load") {
            public void run() {
                loadFromDisk();
            }
        }.start();
        FutureTask<Map<String, Object>> futureTask = new FutureTask<>(() -> loadFromDisk());
        mMap = futureTask;
        new Thread(futureTask, "SharedPreferencesImpl-load").start();
    }

    private void loadFromDisk() {
    private Map<String, Object> loadFromDisk() {
        synchronized (mLock) {
            if (mLoaded) {
                return;
            }
            if (mBackupFile.exists()) {
                mFile.delete();
                mBackupFile.renameTo(mFile);
@@ -158,16 +150,14 @@ final class SharedPreferencesImpl implements SharedPreferences {
        }

        synchronized (mLock) {
            mLoaded = true;
            if (map != null) {
                mMap = map;
                mStatTimestamp = stat.st_mtim;
                mStatSize = stat.st_size;
            } else {
                mMap = new HashMap<>();
                map = new HashMap<>();
            }
            mLock.notifyAll();
        }
        return map;
    }

    static File makeBackupFile(File prefsFile) {
@@ -226,36 +216,37 @@ final class SharedPreferencesImpl implements SharedPreferences {
        }
    }

    private void awaitLoadedLocked() {
        if (!mLoaded) {
    private @GuardedBy("mLock") Map<String, Object> getLoaded() {
        try {
            return mMap.get();
        } catch (InterruptedException | ExecutionException e) {
            throw new IllegalStateException(e);
        }
    }
    private @GuardedBy("mLock") Map<String, Object> getLoadedWithBlockGuard() {
        if (!mMap.isDone()) {
            // Raise an explicit StrictMode onReadFromDisk for this
            // thread, since the real read will be in a different
            // thread and otherwise ignored by StrictMode.
            BlockGuard.getThreadPolicy().onReadFromDisk();
        }
        while (!mLoaded) {
            try {
                mLock.wait();
            } catch (InterruptedException unused) {
            }
        }
        return getLoaded();
    }

    @Override
    public Map<String, ?> getAll() {
        Map<String, Object> map = getLoadedWithBlockGuard();
        synchronized (mLock) {
            awaitLoadedLocked();
            //noinspection unchecked
            return new HashMap<String, Object>(mMap);
            return new HashMap<String, Object>(map);
        }
    }

    @Override
    @Nullable
    public String getString(String key, @Nullable String defValue) {
        Map<String, Object> map = getLoadedWithBlockGuard();
        synchronized (mLock) {
            awaitLoadedLocked();
            String v = (String)mMap.get(key);
            String v = (String) map.get(key);
            return v != null ? v : defValue;
        }
    }
@@ -263,66 +254,65 @@ final class SharedPreferencesImpl implements SharedPreferences {
    @Override
    @Nullable
    public Set<String> getStringSet(String key, @Nullable Set<String> defValues) {
        Map<String, Object> map = getLoadedWithBlockGuard();
        synchronized (mLock) {
            awaitLoadedLocked();
            Set<String> v = (Set<String>) mMap.get(key);
            @SuppressWarnings("unchecked")
            Set<String> v = (Set<String>) map.get(key);
            return v != null ? v : defValues;
        }
    }

    @Override
    public int getInt(String key, int defValue) {
        Map<String, Object> map = getLoadedWithBlockGuard();
        synchronized (mLock) {
            awaitLoadedLocked();
            Integer v = (Integer)mMap.get(key);
            Integer v = (Integer) map.get(key);
            return v != null ? v : defValue;
        }
    }
    @Override
    public long getLong(String key, long defValue) {
        Map<String, Object> map = getLoadedWithBlockGuard();
        synchronized (mLock) {
            awaitLoadedLocked();
            Long v = (Long)mMap.get(key);
            Long v = (Long) map.get(key);
            return v != null ? v : defValue;
        }
    }
    @Override
    public float getFloat(String key, float defValue) {
        Map<String, Object> map = getLoadedWithBlockGuard();
        synchronized (mLock) {
            awaitLoadedLocked();
            Float v = (Float)mMap.get(key);
            Float v = (Float) map.get(key);
            return v != null ? v : defValue;
        }
    }
    @Override
    public boolean getBoolean(String key, boolean defValue) {
        Map<String, Object> map = getLoadedWithBlockGuard();
        synchronized (mLock) {
            awaitLoadedLocked();
            Boolean v = (Boolean)mMap.get(key);
            Boolean v = (Boolean) map.get(key);
            return v != null ? v : defValue;
        }
    }

    @Override
    public boolean contains(String key) {
        Map<String, Object> map = getLoadedWithBlockGuard();
        synchronized (mLock) {
            awaitLoadedLocked();
            return mMap.containsKey(key);
            return map.containsKey(key);
        }
    }

    @Override
    public Editor edit() {
        // TODO: remove the need to call awaitLoadedLocked() when
        // TODO: remove the need to call getLoaded() when
        // requesting an editor.  will require some work on the
        // Editor, but then we should be able to do:
        //
        //      context.getSharedPreferences(..).edit().putString(..).apply()
        //
        // ... all without blocking.
        synchronized (mLock) {
            awaitLoadedLocked();
        }
        getLoadedWithBlockGuard();

        return new EditorImpl();
    }
@@ -476,13 +466,43 @@ final class SharedPreferencesImpl implements SharedPreferences {
                // a memory commit comes in when we're already
                // writing to disk.
                if (mDiskWritesInFlight > 0) {
                    // We can't modify our mMap as a currently
                    // We can't modify our map as a currently
                    // in-flight write owns it.  Clone it before
                    // modifying it.
                    // noinspection unchecked
                    mMap = new HashMap<String, Object>(mMap);
                    mMap = new Future<Map<String, Object>>() {
                        private Map<String, Object> mCopiedMap =
                                new HashMap<String, Object>(getLoaded());

                        @Override
                        public boolean cancel(boolean mayInterruptIfRunning) {
                            return false;
                        }

                        @Override
                        public boolean isCancelled() {
                            return false;
                        }

                        @Override
                        public boolean isDone() {
                            return true;
                        }

                        @Override
                        public Map<String, Object> get()
                                throws InterruptedException, ExecutionException {
                            return mCopiedMap;
                        }

                        @Override
                        public Map<String, Object> get(long timeout, TimeUnit unit)
                                throws InterruptedException, ExecutionException, TimeoutException {
                            return mCopiedMap;
                        }
                    };
                }
                mapToWriteToDisk = mMap;
                mapToWriteToDisk = getLoaded();
                mDiskWritesInFlight++;

                boolean hasListeners = mListeners.size() > 0;