[Bug] Possible data loss in SavePropertiesAsync()

[ypresto]

Description

In Xamarin.Forms.Platform.{iOS,Android,Tizen}/Deserializer.cs, SerializePropertiesAsync() (called by Application.Current.SavePropertiesAsync()) saves file in below procedure. So if application died (e.g. killed by OS) in middle of 2 and 3, it will cause data loss.

1. Write to tmp file
2. Delete primary file
3. Move tmp file to path of primary file

if (store.FileExists(PropertyStoreFile))
	store.DeleteFile(PropertyStoreFile);
store.MoveFile(PropertyStoreFile + ".tmp", PropertyStoreFile);

Also, Xamarin.Forms.Platform.WPF/Deserializer.cs is overwriting primary file directly, so it can also causes data loss if app died while writing to that file (half-written).

Steps to Reproduce

1. Prepare application which calls SavePropertiesAsync() (I used COCOA (COVID-19 Contact-Confirming Application of Japan), but it need complicated setup. I'll upload minimal app if it is necessary)
2. Add root directory of Xamarin.Forms to debug source file list (in solution option of VSMac).
3. Add breakpoint on a line calls SavePropertiesAsync(), then step to SerializePropertiesAsync by clicking step into / step out appropriately. (Note: First step into at SavePropertiesAsync() line will jump to AsyncTaskMethodBuilder, then clicking step into several times will navigate to first line of SavePropertiesAsync() function)
4. Add breakpoint on the line calls store.MoveFile(), then continue.
5. Ensure paused on MoveFile() line, then stop application by clicking square icon on toolbar.
6. Start application.

Expected Behavior

Application.Current.Properties should not be lost.

Actual Behavior

You'll get empty Application.Current.Properties as the store file is deleted.

Basic Information

• Version with issue: Tested with 4.6.0.967 but the buggy code still used in 5.0.0 branch.
• Last known good version: not sure
• Platform Target Frameworks:
  • iOS: 14.4
  • Android: not tested but using same logic
  • WPF: not the same logic but possible data loss with half-written file

Reproduction Link

None. But several case of random unexpected reset of the COVID iOS app are reported.
cocoa-mhlw/cocoa#16 (comment)

Workaround

Add below code and call it before accessing Properties.
Note that checking for non-existence or empty of "PropertyStore.forms" before moving tmp file is very important, because deletion always happens after file is flushed without error (no half-written file).

        private void RecoverLostPropertiesFile(ILoggerService LoggerService)
        {
            const string PropertyStoreFile = "PropertyStore.forms";
            const string PropertyStoreTmpFile = PropertyStoreFile + ".tmp";
            var store = IsolatedStorageFile.GetUserStoreForApplication();
            if (store.FileExists(PropertyStoreTmpFile))
            {
                if (store.FileExists(PropertyStoreFile))
                {
                    // Empty file could be exist because current impl of Xamarin.Forms uses System.IO.FileMode.OpenOrCreate for reading file.
                    using (var stream = store.OpenFile(PropertyStoreFile, System.IO.FileMode.Open))
                    {
                        if (stream.Length > 0)
                        {
                            // tmp file exists, but the store file contains data.
                            return;
                        }
                    }
                    // delete empty file
                    store.DeleteFile(PropertyStoreFile);
                }
                // tmp file exists while primary file is deleted.
                // It means tmp file is fully written (before store.DeleteFile()).

                // TODO: log warning ($"{PropertyStoreFile} is not found or empty, but tmp file {PropertyStoreTmpFile} is found. Recovering from tmp file.")
                store.MoveFile(PropertyStoreTmpFile, PropertyStoreFile);
            }
        }

Possible solution

SharedPreferences in Android handles backup file well.

1. Move primary file to backup path
2. Write file to primary file path
3. Delete backup file

So if backup file exists primary file can be half-written, other hand non-existent of backup means that primary file was written without error.

https://android.googlesource.com/platform/frameworks/base.git/+/android-11.0.0_r31/core/java/android/app/SharedPreferencesImpl.java#152