[shared_preferences] Fix initialization race (#4159)

During the NNBD transition, the structure of the completer logic in the initialization flow was incorrectly changed to set the field after an `await` instead of immediately.

Also updates the error handling to handle `Error` the same way it currently handles `Exception`, which this change surfaced.

Fixes https://github.com/flutter/flutter/issues/42407

packages/shared_preferences/shared_preferences/CHANGELOG.md

@@ -1,5 +1,7 @@
-## NEXT
+## 2.1.2
 
+* Fixes singleton initialization race condition introduced during NNBD
+  transition.
 * Updates minimum supported macOS version to 10.14.
 * Updates minimum supported SDK version to Flutter 3.3/Dart 2.18.
 

packages/shared_preferences/shared_preferences/lib/shared_preferences.dart

@@ -62,11 +62,12 @@ class SharedPreferences {
     if (_completer == null) {
       final Completer<SharedPreferences> completer =
           Completer<SharedPreferences>();
+      _completer = completer;
       try {
         final Map<String, Object> preferencesMap =
             await _getSharedPreferencesMap();
         completer.complete(SharedPreferences._(preferencesMap));
-      } on Exception catch (e) {
+      } catch (e) {
         // If there's an error, explicitly return the future with an error.
         // then set the completer to null so we can retry.
         completer.completeError(e);
@@ -74,7 +75,6 @@ class SharedPreferences {
         _completer = null;
         return sharedPrefsFuture;
       }
-      _completer = completer;
     }
     return _completer!.future;
   }

packages/shared_preferences/shared_preferences/pubspec.yaml

@@ -3,7 +3,7 @@ description: Flutter plugin for reading and writing simple key-value pairs.
   Wraps NSUserDefaults on iOS and SharedPreferences on Android.
 repository: https://github.com/flutter/packages/tree/main/packages/shared_preferences/shared_preferences
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+shared_preferences%22
-version: 2.1.1
+version: 2.1.2
 
 environment:
   sdk: ">=2.18.0 <4.0.0"

packages/shared_preferences/shared_preferences/test/shared_preferences_test.dart

@@ -10,199 +10,196 @@ import 'package:shared_preferences_platform_interface/shared_preferences_platfor
 void main() {
   TestWidgetsFlutterBinding.ensureInitialized();
 
-  group('SharedPreferences', () {
-    const String testString = 'hello world';
-    const bool testBool = true;
-    const int testInt = 42;
-    const double testDouble = 3.14159;
-    const List<String> testList = <String>['foo', 'bar'];
-    const Map<String, Object> testValues = <String, Object>{
-      'flutter.String': testString,
-      'flutter.bool': testBool,
-      'flutter.int': testInt,
-      'flutter.double': testDouble,
-      'flutter.List': testList,
-    };
+  const String testString = 'hello world';
+  const bool testBool = true;
+  const int testInt = 42;
+  const double testDouble = 3.14159;
+  const List<String> testList = <String>['foo', 'bar'];
+  const Map<String, Object> testValues = <String, Object>{
+    'flutter.String': testString,
+    'flutter.bool': testBool,
+    'flutter.int': testInt,
+    'flutter.double': testDouble,
+    'flutter.List': testList,
+  };
 
-    const String testString2 = 'goodbye world';
-    const bool testBool2 = false;
-    const int testInt2 = 1337;
-    const double testDouble2 = 2.71828;
-    const List<String> testList2 = <String>['baz', 'quox'];
-    const Map<String, dynamic> testValues2 = <String, dynamic>{
-      'flutter.String': testString2,
-      'flutter.bool': testBool2,
-      'flutter.int': testInt2,
-      'flutter.double': testDouble2,
-      'flutter.List': testList2,
-    };
+  const String testString2 = 'goodbye world';
+  const bool testBool2 = false;
+  const int testInt2 = 1337;
+  const double testDouble2 = 2.71828;
+  const List<String> testList2 = <String>['baz', 'quox'];
+  const Map<String, dynamic> testValues2 = <String, dynamic>{
+    'flutter.String': testString2,
+    'flutter.bool': testBool2,
+    'flutter.int': testInt2,
+    'flutter.double': testDouble2,
+    'flutter.List': testList2,
+  };
 
-    late FakeSharedPreferencesStore store;
-    late SharedPreferences preferences;
+  late FakeSharedPreferencesStore store;
+  late SharedPreferences preferences;
 
-    setUp(() async {
-      store = FakeSharedPreferencesStore(testValues);
-      SharedPreferencesStorePlatform.instance = store;
-      preferences = await SharedPreferences.getInstance();
-      store.log.clear();
-    });
+  setUp(() async {
+    store = FakeSharedPreferencesStore(testValues);
+    SharedPreferencesStorePlatform.instance = store;
+    preferences = await SharedPreferences.getInstance();
+    store.log.clear();
+  });
 
-    test('reading', () async {
-      expect(preferences.get('String'), testString);
-      expect(preferences.get('bool'), testBool);
-      expect(preferences.get('int'), testInt);
-      expect(preferences.get('double'), testDouble);
-      expect(preferences.get('List'), testList);
-      expect(preferences.getString('String'), testString);
-      expect(preferences.getBool('bool'), testBool);
-      expect(preferences.getInt('int'), testInt);
-      expect(preferences.getDouble('double'), testDouble);
-      expect(preferences.getStringList('List'), testList);
-      expect(store.log, <Matcher>[]);
-    });
+  test('reading', () async {
+    expect(preferences.get('String'), testString);
+    expect(preferences.get('bool'), testBool);
+    expect(preferences.get('int'), testInt);
+    expect(preferences.get('double'), testDouble);
+    expect(preferences.get('List'), testList);
+    expect(preferences.getString('String'), testString);
+    expect(preferences.getBool('bool'), testBool);
+    expect(preferences.getInt('int'), testInt);
+    expect(preferences.getDouble('double'), testDouble);
+    expect(preferences.getStringList('List'), testList);
+    expect(store.log, <Matcher>[]);
+  });
 
-    test('writing', () async {
-      await Future.wait(<Future<bool>>[
-        preferences.setString('String', testString2),
-        preferences.setBool('bool', testBool2),
-        preferences.setInt('int', testInt2),
-        preferences.setDouble('double', testDouble2),
-        preferences.setStringList('List', testList2)
-      ]);
-      expect(
+  test('writing', () async {
+    await Future.wait(<Future<bool>>[
+      preferences.setString('String', testString2),
+      preferences.setBool('bool', testBool2),
+      preferences.setInt('int', testInt2),
+      preferences.setDouble('double', testDouble2),
+      preferences.setStringList('List', testList2)
+    ]);
+    expect(
+      store.log,
+      <Matcher>[
+        isMethodCall('setValue', arguments: <dynamic>[
+          'String',
+          'flutter.String',
+          testString2,
+        ]),
+        isMethodCall('setValue', arguments: <dynamic>[
+          'Bool',
+          'flutter.bool',
+          testBool2,
+        ]),
+        isMethodCall('setValue', arguments: <dynamic>[
+          'Int',
+          'flutter.int',
+          testInt2,
+        ]),
+        isMethodCall('setValue', arguments: <dynamic>[
+          'Double',
+          'flutter.double',
+          testDouble2,
+        ]),
+        isMethodCall('setValue', arguments: <dynamic>[
+          'StringList',
+          'flutter.List',
+          testList2,
+        ]),
+      ],
+    );
+    store.log.clear();
+
+    expect(preferences.getString('String'), testString2);
+    expect(preferences.getBool('bool'), testBool2);
+    expect(preferences.getInt('int'), testInt2);
+    expect(preferences.getDouble('double'), testDouble2);
+    expect(preferences.getStringList('List'), testList2);
+    expect(store.log, equals(<MethodCall>[]));
+  });
+
+  test('removing', () async {
+    const String key = 'testKey';
+    await preferences.remove(key);
+    expect(
         store.log,
-        <Matcher>[
-          isMethodCall('setValue', arguments: <dynamic>[
-            'String',
-            'flutter.String',
-            testString2,
-          ]),
-          isMethodCall('setValue', arguments: <dynamic>[
-            'Bool',
-            'flutter.bool',
-            testBool2,
-          ]),
-          isMethodCall('setValue', arguments: <dynamic>[
-            'Int',
-            'flutter.int',
-            testInt2,
-          ]),
-          isMethodCall('setValue', arguments: <dynamic>[
-            'Double',
-            'flutter.double',
-            testDouble2,
-          ]),
-          isMethodCall('setValue', arguments: <dynamic>[
-            'StringList',
-            'flutter.List',
-            testList2,
-          ]),
-        ],
-      );
-      store.log.clear();
+        List<Matcher>.filled(
+          1,
+          isMethodCall(
+            'remove',
+            arguments: 'flutter.$key',
+          ),
+          growable: true,
+        ));
+  });
 
-      expect(preferences.getString('String'), testString2);
-      expect(preferences.getBool('bool'), testBool2);
-      expect(preferences.getInt('int'), testInt2);
-      expect(preferences.getDouble('double'), testDouble2);
-      expect(preferences.getStringList('List'), testList2);
-      expect(store.log, equals(<MethodCall>[]));
+  test('containsKey', () async {
+    const String key = 'testKey';
+
+    expect(false, preferences.containsKey(key));
+
+    await preferences.setString(key, 'test');
+    expect(true, preferences.containsKey(key));
+  });
+
+  test('clearing', () async {
+    await preferences.clear();
+    expect(preferences.getString('String'), null);
+    expect(preferences.getBool('bool'), null);
+    expect(preferences.getInt('int'), null);
+    expect(preferences.getDouble('double'), null);
+    expect(preferences.getStringList('List'), null);
+    expect(store.log, <Matcher>[isMethodCall('clear', arguments: null)]);
+  });
+
+  test('reloading', () async {
+    await preferences.setString('String', testString);
+    expect(preferences.getString('String'), testString);
+
+    SharedPreferences.setMockInitialValues(testValues2.cast<String, Object>());
+    expect(preferences.getString('String'), testString);
+
+    await preferences.reload();
+    expect(preferences.getString('String'), testString2);
+  });
+
+  test('back to back calls should return same instance.', () async {
+    final Future<SharedPreferences> first = SharedPreferences.getInstance();
+    final Future<SharedPreferences> second = SharedPreferences.getInstance();
+    expect(await first, await second);
+  });
+
+  test('string list type is dynamic (usually from method channel)', () async {
+    SharedPreferences.setMockInitialValues(<String, Object>{
+      'dynamic_list': <dynamic>['1', '2']
     });
+    final SharedPreferences prefs = await SharedPreferences.getInstance();
+    final List<String>? value = prefs.getStringList('dynamic_list');
+    expect(value, <String>['1', '2']);
+  });
 
-    test('removing', () async {
-      const String key = 'testKey';
-      await preferences.remove(key);
-      expect(
-          store.log,
-          List<Matcher>.filled(
-            1,
-            isMethodCall(
-              'remove',
-              arguments: 'flutter.$key',
-            ),
-            growable: true,
-          ));
-    });
-
-    test('containsKey', () async {
-      const String key = 'testKey';
-
-      expect(false, preferences.containsKey(key));
-
-      await preferences.setString(key, 'test');
-      expect(true, preferences.containsKey(key));
-    });
-
-    test('clearing', () async {
-      await preferences.clear();
-      expect(preferences.getString('String'), null);
-      expect(preferences.getBool('bool'), null);
-      expect(preferences.getInt('int'), null);
-      expect(preferences.getDouble('double'), null);
-      expect(preferences.getStringList('List'), null);
-      expect(store.log, <Matcher>[isMethodCall('clear', arguments: null)]);
-    });
-
-    test('reloading', () async {
-      await preferences.setString('String', testString);
-      expect(preferences.getString('String'), testString);
+  group('mocking', () {
+    const String key = 'dummy';
+    const String prefixedKey = 'flutter.$key';
 
+    test('test 1', () async {
       SharedPreferences.setMockInitialValues(
-          testValues2.cast<String, Object>());
-      expect(preferences.getString('String'), testString);
-
-      await preferences.reload();
-      expect(preferences.getString('String'), testString2);
-    });
-
-    test('back to back calls should return same instance.', () async {
-      final Future<SharedPreferences> first = SharedPreferences.getInstance();
-      final Future<SharedPreferences> second = SharedPreferences.getInstance();
-      expect(await first, await second);
-    });
-
-    test('string list type is dynamic (usually from method channel)', () async {
-      SharedPreferences.setMockInitialValues(<String, Object>{
-        'dynamic_list': <dynamic>['1', '2']
-      });
+          <String, Object>{prefixedKey: 'my string'});
       final SharedPreferences prefs = await SharedPreferences.getInstance();
-      final List<String>? value = prefs.getStringList('dynamic_list');
-      expect(value, <String>['1', '2']);
+      final String? value = prefs.getString(key);
+      expect(value, 'my string');
     });
 
-    group('mocking', () {
-      const String key = 'dummy';
-      const String prefixedKey = 'flutter.$key';
-
-      test('test 1', () async {
-        SharedPreferences.setMockInitialValues(
-            <String, Object>{prefixedKey: 'my string'});
-        final SharedPreferences prefs = await SharedPreferences.getInstance();
-        final String? value = prefs.getString(key);
-        expect(value, 'my string');
-      });
-
-      test('test 2', () async {
-        SharedPreferences.setMockInitialValues(
-            <String, Object>{prefixedKey: 'my other string'});
-        final SharedPreferences prefs = await SharedPreferences.getInstance();
-        final String? value = prefs.getString(key);
-        expect(value, 'my other string');
-      });
+    test('test 2', () async {
+      SharedPreferences.setMockInitialValues(
+          <String, Object>{prefixedKey: 'my other string'});
+      final SharedPreferences prefs = await SharedPreferences.getInstance();
+      final String? value = prefs.getString(key);
+      expect(value, 'my other string');
     });
+  });
 
-    test('writing copy of strings list', () async {
-      final List<String> myList = <String>[];
-      await preferences.setStringList('myList', myList);
-      myList.add('foobar');
+  test('writing copy of strings list', () async {
+    final List<String> myList = <String>[];
+    await preferences.setStringList('myList', myList);
+    myList.add('foobar');
 
-      final List<String> cachedList = preferences.getStringList('myList')!;
-      expect(cachedList, <String>[]);
+    final List<String> cachedList = preferences.getStringList('myList')!;
+    expect(cachedList, <String>[]);
 
-      cachedList.add('foobar2');
+    cachedList.add('foobar2');
 
-      expect(preferences.getStringList('myList'), <String>[]);
-    });
+    expect(preferences.getStringList('myList'), <String>[]);
   });
 
   test('calling mock initial values with non-prefixed keys succeeds', () async {
@@ -214,6 +211,16 @@ void main() {
     expect(value, 'foo');
   });
 
+  test('getInstance always returns the same instance', () async {
+    SharedPreferencesStorePlatform.instance = SlowInitSharedPreferencesStore();
+
+    final Future<SharedPreferences> firstFuture =
+        SharedPreferences.getInstance();
+    final Future<SharedPreferences> secondFuture =
+        SharedPreferences.getInstance();
+    expect(identical(await firstFuture, await secondFuture), true);
+  });
+
   test('calling setPrefix after getInstance throws', () async {
     const String newPrefix = 'newPrefix';
 
@@ -367,6 +374,15 @@ class UnimplementedSharedPreferencesStore
   }
 }
 
+class SlowInitSharedPreferencesStore
+    extends UnimplementedSharedPreferencesStore {
+  @override
+  Future<Map<String, Object>> getAll() async {
+    await Future<void>.delayed(const Duration(seconds: 1));
+    return <String, Object>{};
+  }
+}
+
 class ThrowingSharedPreferencesStore extends SharedPreferencesStorePlatform {
   @override
   Future<bool> clear() {