Merge "Fix a race between requesting/loading/writing SharedPreferences." into gingerbread (64591009) · Commits · e / os / android_frameworks_base

core/java/android/app/ContextImpl.java

+63 −41

Original line number	Original line	Diff line number	Diff line
	@@ -337,31 +337,41 @@ class ContextImpl extends Context {
	@Override		@Override
	public SharedPreferences getSharedPreferences(String name, int mode) {		public SharedPreferences getSharedPreferences(String name, int mode) {
	SharedPreferencesImpl sp;		SharedPreferencesImpl sp;
			File prefsFile;
			boolean needInitialLoad = false;
	synchronized (sSharedPrefs) {		synchronized (sSharedPrefs) {
	sp = sSharedPrefs.get(name);		sp = sSharedPrefs.get(name);
	if (sp != null && !sp.hasFileChanged()) {		if (sp != null && !sp.hasFileChangedUnexpectedly()) {
	//Log.i(TAG, "Returning existing prefs " + name + ": " + sp);
	return sp;		return sp;
	}		}
			prefsFile = getSharedPrefsFile(name);
			if (sp == null) {
			sp = new SharedPreferencesImpl(prefsFile, mode, null);
			sSharedPrefs.put(name, sp);
			needInitialLoad = true;
			}
	}		}
	File f = getSharedPrefsFile(name);

	FileInputStream str = null;		synchronized (sp) {
	File backup = makeBackupFile(f);		if (needInitialLoad && sp.isLoaded()) {
			// lost the race to load; another thread handled it
			return sp;
			}
			File backup = makeBackupFile(prefsFile);
	if (backup.exists()) {		if (backup.exists()) {
	f.delete();		prefsFile.delete();
	backup.renameTo(f);		backup.renameTo(prefsFile);
	}		}

	// Debugging		// Debugging
	if (f.exists() && !f.canRead()) {		if (prefsFile.exists() && !prefsFile.canRead()) {
	Log.w(TAG, "Attempt to read preferences file " + f + " without permission");		Log.w(TAG, "Attempt to read preferences file " + prefsFile + " without permission");
	}		}

	Map map = null;		Map map = null;
	if (f.exists() && f.canRead()) {		if (prefsFile.exists() && prefsFile.canRead()) {
	try {		try {
	str = new FileInputStream(f);		FileInputStream str = new FileInputStream(prefsFile);
	map = XmlUtils.readMapXml(str);		map = XmlUtils.readMapXml(str);
	str.close();		str.close();
	} catch (org.xmlpull.v1.XmlPullParserException e) {		} catch (org.xmlpull.v1.XmlPullParserException e) {
	@@ -372,21 +382,10 @@ class ContextImpl extends Context {
	Log.w(TAG, "getSharedPreferences", e);		Log.w(TAG, "getSharedPreferences", e);
	}		}
	}		}

	synchronized (sSharedPrefs) {
	if (sp != null) {
	//Log.i(TAG, "Updating existing prefs " + name + " " + sp + ": " + map);
	sp.replace(map);		sp.replace(map);
	} else {
	sp = sSharedPrefs.get(name);
	if (sp == null) {
	sp = new SharedPreferencesImpl(f, mode, map);
	sSharedPrefs.put(name, sp);
	}
	}		}
	return sp;		return sp;
	}		}
	}

	private File getPreferencesDir() {		private File getPreferencesDir() {
	synchronized (mSync) {		synchronized (mSync) {
	@@ -2712,6 +2711,10 @@ class ContextImpl extends Context {

	private static final class SharedPreferencesImpl implements SharedPreferences {		private static final class SharedPreferencesImpl implements SharedPreferences {

			// Lock ordering rules:
			// - acquire SharedPreferencesImpl.this before EditorImpl.this
			// - acquire mWritingToDiskLock before EditorImpl.this

	private final File mFile;		private final File mFile;
	private final File mBackupFile;		private final File mBackupFile;
	private final int mMode;		private final int mMode;
	@@ -2719,6 +2722,7 @@ class ContextImpl extends Context {
	private Map<String, Object> mMap; // guarded by 'this'		private Map<String, Object> mMap; // guarded by 'this'
	private long mTimestamp; // guarded by 'this'		private long mTimestamp; // guarded by 'this'
	private int mDiskWritesInFlight = 0; // guarded by 'this'		private int mDiskWritesInFlight = 0; // guarded by 'this'
			private boolean mLoaded = false; // guarded by 'this'

	private final Object mWritingToDiskLock = new Object();		private final Object mWritingToDiskLock = new Object();
	private static final Object mContent = new Object();		private static final Object mContent = new Object();
	@@ -2729,6 +2733,7 @@ class ContextImpl extends Context {
	mFile = file;		mFile = file;
	mBackupFile = makeBackupFile(file);		mBackupFile = makeBackupFile(file);
	mMode = mode;		mMode = mode;
			mLoaded = initialContents != null;
	mMap = initialContents != null ? initialContents : new HashMap<String, Object>();		mMap = initialContents != null ? initialContents : new HashMap<String, Object>();
	FileStatus stat = new FileStatus();		FileStatus stat = new FileStatus();
	if (FileUtils.getFileStatus(file.getPath(), stat)) {		if (FileUtils.getFileStatus(file.getPath(), stat)) {
	@@ -2737,7 +2742,23 @@ class ContextImpl extends Context {
	mListeners = new WeakHashMap<OnSharedPreferenceChangeListener, Object>();		mListeners = new WeakHashMap<OnSharedPreferenceChangeListener, Object>();
	}		}

	public boolean hasFileChanged() {		// Has this SharedPreferences ever had values assigned to it?
			boolean isLoaded() {
			synchronized (this) {
			return mLoaded;
			}
			}

			// Has the file changed out from under us? i.e. writes that
			// we didn't instigate.
			public boolean hasFileChangedUnexpectedly() {
			synchronized (this) {
			if (mDiskWritesInFlight > 0) {
			// If we know we caused it, it's not unexpected.
			Log.d(TAG, "disk write in flight, not unexpected.");
			return false;
			}
			}
	FileStatus stat = new FileStatus();		FileStatus stat = new FileStatus();
	if (!FileUtils.getFileStatus(mFile.getPath(), stat)) {		if (!FileUtils.getFileStatus(mFile.getPath(), stat)) {
	return true;		return true;
	@@ -2748,8 +2769,9 @@ class ContextImpl extends Context {
	}		}

	public void replace(Map newContents) {		public void replace(Map newContents) {
	if (newContents != null) {
	synchronized (this) {		synchronized (this) {
			mLoaded = true;
			if (newContents != null) {
	mMap = newContents;		mMap = newContents;
	}		}
	}		}