Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[shared_preferences] [windows,linux] Data can be corrupted due to lack of atomic file writes #89211

Open
kirill-21 opened this issue Aug 31, 2021 · 27 comments
Assignees
Labels
a: desktop Running on desktop fyi-windows For the attention of the Windows platform team Bot is counting down the days until it unassigns the issue p: shared_preferences Plugin to read and write Shared Preferences P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels. platform-linux Building on or for Linux specifically platform-windows Building on or for Windows specifically team-ecosystem Owned by Ecosystem team triaged-ecosystem Triaged by Ecosystem team

Comments

@kirill-21
Copy link

kirill-21 commented Aug 31, 2021

 SharedPreferences _preferences = await SharedPreferences.getInstance();

This code simply fails with an error if config file is broken. Maybe it's good to clear preferences file if it contains an error. I do not know why the preferences file breaks but since i've published my app, there were some users, who reported this issue.

Platform where i faced this issue: Windows.

Flutter version: 2.2.3 from stable channel.

I have no idea why but file becomes something like this
image

@darshankawar darshankawar added the in triage Presently being triaged by the triage team label Aug 31, 2021
@darshankawar
Copy link
Member

@kirill-21
Please provide a complete minimal code sample.
Also can you elaborate on if config file is broken. with an example ?

@darshankawar darshankawar added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Aug 31, 2021
@kirill-21
Copy link
Author

kirill-21 commented Aug 31, 2021

@kirill-21
Please provide a complete minimal code sample.
Also can you elaborate on if config file is broken. with an example ?

Rename .txt -> .json (github does not allow to upload .json) (if you open it with notepad++, you'll see a lot of NUL NUL NUL NUL...)
shared_preferences.txt

add this to dependencies

dependencies:
  shared_preferences: ^2.0.7

place renamed shared preferences file to your's application folder (%appdata%/com.yourname/yourname [Example: C:\Users\user\AppData\Roaming\com.gskinner\flutter_folio\shared_preferences.json]) and add this into the main() function

SharedPreferences _preferences = await SharedPreferences.getInstance();

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Aug 31, 2021
@darshankawar
Copy link
Member

@kirill-21
Sorry where do I need to put the file again ?
I renamed it to shared_preferences.json.

@darshankawar darshankawar added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Aug 31, 2021
@kirill-21
Copy link
Author

@kirill-21
Sorry where do I need to put the file again ?
I renamed it to shared_preferences.json.

%appdata%/com.yourname/yourname
com.yourname you specify when you create a flutter project
yourname you specify when you create a flutter project

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Aug 31, 2021
@darshankawar
Copy link
Member

That seems to be a Windows path. I am using Mac and tried putting the file in project's root path and called SharedPreferences _preferences = await SharedPreferences.getInstance(); in main() function, but didn't see the issue. Maybe something is still missing.
Please be descriptive while providing details.

@darshankawar darshankawar added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Aug 31, 2021
@kirill-21
Copy link
Author

kirill-21 commented Aug 31, 2021

That seems to be a Windows path. I am using Mac and tried putting the file in project's root path and called SharedPreferences _preferences = await SharedPreferences.getInstance(); in main() function, but didn't see the issue. Maybe something is still missing.
Please be descriptive while providing details.

I have no idea where settings files for application is stored on macOs, but i specified, that i face issue on Windows

When you call

SharedPreferences _preferences = await SharedPreferences.getInstance();

method(and save at least one parameter), shared preferences creates a shared_preferences file on your device and stores all setting there. Find this file and replace it with the provided one. On windows this file is stored in %appdata%/com.yourdeveloper/yourappname
image

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Aug 31, 2021
@stuartmorgan
Copy link
Contributor

I am using Mac and tried [...]

Windows and macOS have completely unrelated implementations of preference storage. You cannot reproduce a Windows storage issue on macOS regardless of where you try to put that file.

@stuartmorgan stuartmorgan added a: desktop Running on desktop P3 Issues that are less important to the Flutter project p: first party p: shared_preferences Plugin to read and write Shared Preferences platform-windows Building on or for Windows specifically labels Aug 31, 2021
@stuartmorgan
Copy link
Contributor

stuartmorgan commented Aug 31, 2021

This code simply fails with an error if config file is broken.

That seems like it's likely the correct behavior, but we'd need to document that and provide a reset option.

Maybe it's good to clear preferences file if it contains an error.

Doing so automatically would give developers no chance to explain to users that their settings are lost, do some other fallback, etc. That doesn't seem desirable.

I do not know why the preferences file breaks

Is there anything unusual about how you are using prefs?

The other thing we could consider is doing an atomic write (save then move) if Dart doesn't do that internally.

@kirill-21
Copy link
Author

This code simply fails with an error if config file is broken.

That seems like it's likely the correct behavior, but we'd need to document that and provide a reset option.

Maybe it's good to clear preferences file if it contains an error.

Doing so automatically would give developers no chance to explain to users that their settings are lost, do some other fallback, etc. That doesn't seem desirable.

I do not know why the preferences file breaks

Is there anything unusual about how you are using prefs?

The other thing we could consider is doing an atomic write (save them move) if Dart doesn't do that internally.

You can create a backup copy of shared_preferences file on each call of getInstance() method. The next time app tries to call it again and catches an error - delete broken shared_preferences file and restore a backup copy with almost up-to-date settings.

"Is there anything unusual about how you are using prefs?" - No, i've questioned users, who faced this issue and many of them told that they did not even change any settings in the app. And while the app was in the development stage(that's something around 5 months), i've never faced this issue.

"That seems like it's likely the correct behavior" - but there is no way to clear preferences before calling getInstance() method.

@stuartmorgan
Copy link
Contributor

"That seems like it's likely the correct behavior" - but there is no way to clear preferences before calling getInstance() method.

Please read the whole sentence, not just the half that you quoted.

@darshankawar darshankawar added passed first triage and removed in triage Presently being triaged by the triage team labels Sep 1, 2021
@kirill-21
Copy link
Author

kirill-21 commented Sep 1, 2021

"That seems like it's likely the correct behavior" - but there is no way to clear preferences before calling getInstance() method.

Please read the whole sentence, not just the half that you quoted.

I've tried to make a temporary solution by making a backup copy of settings right after each getInstance() call. After some tests i found out that sometimes this backup copy also creates broken. I guess the break of the file where it fills with nul nul nul nul instead of the actual preferences happens in this getInstance() method.

Do you have any ideas why it happens?
image
image
image

@Maged12
Copy link

Maged12 commented Sep 8, 2021

I have the same issue and this happens to my users a lot and i only tell them to delete the file and open the app again and this is so annoying for them cos they saved data has gone .
So any advice ? or solutions ?

@kirill-21
Copy link
Author

I have the same issue and this happens to my users a lot and i only tell them to delete the file and open the app again and this is so annoying for them cos they saved data has gone .
So any advice ? or solutions ?

You can try to backup the file on each getInstance() successful call and restore it if something went wrong. kAppFolderPath is the path to your application's folder (C:\Users\user\AppData\Roaming\com.test\test)

try {
  _preferences = await SharedPreferences.getInstance();
  
  // Store a copy of user's preferences on the disk
  await _backupPreferences();
} catch (error) {
  // Remove broken preferences files and restore previous settings
  await _restorePreferencesFromBackup();
  _preferences = await SharedPreferences.getInstance();
}

/// Makes a backup copy of the current shared preferences file.
Future<void> _backupPreferences() async {
  try {
    final String original = '$kAppFolderPath\\shared_preferences.json';
    final String backup = '$kAppFolderPath\\shared_preferences_backup.json';

    if (await File(backup).exists()) await File(backup).delete(recursive: true);
    await File(original).copy(backup);
  } catch (_) {
    /* Do nothing */
  }
}

/// Removes current version of shared_preferences file and restores previous
/// user settings from a backup file (if it exists).
Future<void> _restorePreferencesFromBackup() async {
  try {
    final String original = '$kAppFolderPath\\shared_preferences.json';
    final String backup = '$kAppFolderPath\\shared_preferences_backup.json';

    await File(original).delete(recursive: true);

    if (await File(backup).exists()) {
      // Check if current backup copy is not broken by looking for letters and "
      // symbol in it to replace it as an original Settings file
      final String preferences = await File(backup).readAsString();
      if (preferences.contains('"') && preferences.contains(RegExp('[A-z]'))) {
        await File(backup).copy(original);
      }
    }
  } catch (_) {
    /* Do nothing */
  }
}

@gspencergoog gspencergoog removed the a: desktop Running on desktop label Sep 9, 2021
@feimenggo

This comment was marked as off-topic.

@stuartmorgan stuartmorgan added P2 Important issues not at the top of the work list and removed P3 Issues that are less important to the Flutter project labels Aug 30, 2023
@stuartmorgan
Copy link
Contributor

@stuartmorgan Need to set "flush: true"

That doesn't make the write atomic, so would not prevent things like file corruption due to the computer suddenly powering off.

@NickNevzorov
Copy link

@stuartmorgan Need to set "flush: true"

That doesn't make the write atomic, so would not prevent things like file corruption due to the computer suddenly powering off.

But this prevent damaging setting.json while application simply crashes or process terminated by task manager

@stuartmorgan
Copy link
Contributor

But this prevent damaging setting.json while application simply crashes or process terminated by task manager

How? That flag makes the future not return until the output is flushed; the app can crash or be terminated regardless of whether a Dart future is still outstanding.

@kirill-21
Copy link
Author

kirill-21 commented Aug 30, 2023

@stuartmorgan Need to set "flush: true"

That doesn't make the write atomic, so would not prevent things like file corruption due to the computer suddenly powering off.

But this prevent damaging setting.json while application simply crashes or process terminated by task manager

Trust me crashing is definitely much worse then settings file damaging, because with damaging you can simply use a backup copy. I'm using #89211 (comment) this approach for 2 years in a project with large audience and it's working perfectly without issues.

@ahmedElsarag
Copy link

the same problem happens with me, I find out that it's happening after windows update or restart.

@tarrinneal tarrinneal self-assigned this Feb 13, 2024
@cbracken cbracken added team-linux Owned by the Linux platform team fyi-linux For the attention of the Linux platform team and removed team-desktop labels Jun 6, 2024
@flutter-triage-bot flutter-triage-bot bot removed fyi-linux For the attention of the Linux platform team triaged-desktop labels Jun 7, 2024
@flutter-triage-bot
Copy link

The fyi-linux label is redundant with the team-linux label.
The triaged-desktop label is irrelevant if there is no team-desktop label or fyi-desktop label.

@jmagman jmagman added the triaged-linux Triaged by the Linux platform team label Jun 7, 2024
@stuartmorgan stuartmorgan added team-ecosystem Owned by Ecosystem team triaged-ecosystem Triaged by Ecosystem team fyi-windows For the attention of the Windows platform team and removed team-linux Owned by the Linux platform team triaged-linux Triaged by the Linux platform team labels Jun 7, 2024
@flutter-triage-bot flutter-triage-bot bot added the Bot is counting down the days until it unassigns the issue label Oct 11, 2024
@flutter-triage-bot
Copy link

This issue is assigned to @tarrinneal but has had no recent status updates. Please consider unassigning this issue if it is not going to be addressed in the near future. This allows people to have a clearer picture of what work is actually planned. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: desktop Running on desktop fyi-windows For the attention of the Windows platform team Bot is counting down the days until it unassigns the issue p: shared_preferences Plugin to read and write Shared Preferences P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels. platform-linux Building on or for Linux specifically platform-windows Building on or for Windows specifically team-ecosystem Owned by Ecosystem team triaged-ecosystem Triaged by Ecosystem team
Projects
None yet
Development

No branches or pull requests