Fixed AsyncLocalStorage bug

lzpfmh · Jun 22, 2015 · 8663945 · 8663945
1 parent 5b476d0
commit 8663945
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 46 deletions.
diff --git a/Examples/UIExplorer/UIExplorerIntegrationTests/js/AsyncStorageTest.js b/Examples/UIExplorer/UIExplorerIntegrationTests/js/AsyncStorageTest.js
@@ -53,15 +53,15 @@ function expectEqual(lhs, rhs, testname) {
   );
 }
 
-function expectAsyncNoError(err) {
-  expectTrue(err === null, 'Unexpected Async error: ' + JSON.stringify(err));
+function expectAsyncNoError(place, err) {
+  expectTrue(err === null, 'Unexpected error in ' + place + ': ' + JSON.stringify(err));
 }
 
 function testSetAndGet() {
   AsyncStorage.setItem(KEY_1, VAL_1, (err1) => {
-    expectAsyncNoError(err1);
+    expectAsyncNoError('testSetAndGet/setItem', err1);
     AsyncStorage.getItem(KEY_1, (err2, result) => {
-      expectAsyncNoError(err2);
+      expectAsyncNoError('testSetAndGet/getItem', err2);
       expectEqual(result, VAL_1, 'testSetAndGet setItem');
       updateMessage('get(key_1) correctly returned ' + result);
       runTestCase('should get null for missing key', testMissingGet);
@@ -71,7 +71,7 @@ function testSetAndGet() {
 
 function testMissingGet() {
   AsyncStorage.getItem(KEY_2, (err, result) => {
-    expectAsyncNoError(err);
+    expectAsyncNoError('testMissingGet/setItem', err);
     expectEqual(result, null, 'testMissingGet');
     updateMessage('missing get(key_2) correctly returned ' + result);
     runTestCase('check set twice results in a single key', testSetTwice);
@@ -82,7 +82,7 @@ function testSetTwice() {
   AsyncStorage.setItem(KEY_1, VAL_1, ()=>{
     AsyncStorage.setItem(KEY_1, VAL_1, ()=>{
       AsyncStorage.getItem(KEY_1, (err, result) => {
-        expectAsyncNoError(err);
+        expectAsyncNoError('testSetTwice/setItem', err);
         expectEqual(result, VAL_1, 'testSetTwice');
         updateMessage('setTwice worked as expected');
         runTestCase('test removeItem', testRemoveItem);
@@ -95,25 +95,25 @@ function testRemoveItem() {
   AsyncStorage.setItem(KEY_1, VAL_1, ()=>{
     AsyncStorage.setItem(KEY_2, VAL_2, ()=>{
       AsyncStorage.getAllKeys((err, result) => {
-        expectAsyncNoError(err);
+        expectAsyncNoError('testRemoveItem/getAllKeys', err);
         expectTrue(
           result.indexOf(KEY_1) >= 0 && result.indexOf(KEY_2) >= 0,
           'Missing KEY_1 or KEY_2 in ' + '(' + result + ')'
         );
         updateMessage('testRemoveItem - add two items');
         AsyncStorage.removeItem(KEY_1, (err2) => {
-          expectAsyncNoError(err2);
+          expectAsyncNoError('testRemoveItem/removeItem', err2);
           updateMessage('delete successful ');
           AsyncStorage.getItem(KEY_1, (err3, result2) => {
-            expectAsyncNoError(err3);
+            expectAsyncNoError('testRemoveItem/getItem', err3);
             expectEqual(
               result2,
               null,
               'testRemoveItem: key_1 present after delete'
             );
             updateMessage('key properly removed ');
             AsyncStorage.getAllKeys((err4, result3) => {
-             expectAsyncNoError(err4);
+             expectAsyncNoError('testRemoveItem/getAllKeys', err4);
              expectTrue(
                result3.indexOf(KEY_1) === -1,
                'Unexpected: KEY_1 present in ' + result3
@@ -130,11 +130,11 @@ function testRemoveItem() {
 
 function testMerge() {
   AsyncStorage.setItem(KEY_MERGE, JSON.stringify(VAL_MERGE_1), (err1) => {
-    expectAsyncNoError(err1);
+    expectAsyncNoError('testMerge/setItem', err1);
     AsyncStorage.mergeItem(KEY_MERGE, JSON.stringify(VAL_MERGE_2), (err2) => {
-      expectAsyncNoError(err2);
+      expectAsyncNoError('testMerge/mergeItem', err2);
       AsyncStorage.getItem(KEY_MERGE, (err3, result) => {
-        expectAsyncNoError(err3);
+        expectAsyncNoError('testMerge/setItem', err3);
         expectEqual(JSON.parse(result), VAL_MERGE_EXPECT, 'testMerge');
         updateMessage('objects deeply merged\nDone!');
         done();

diff --git a/React/Modules/RCTAsyncLocalStorage.h b/React/Modules/RCTAsyncLocalStorage.h
@@ -32,6 +32,6 @@
 - (void)getAllKeys:(RCTResponseSenderBlock)callback;
 
 // For clearing data when the bridge may not exist, e.g. when logging out.
-+ (NSError *)clearAllData;
++ (void)clearAllData;
 
 @end
diff --git a/React/Modules/RCTAsyncLocalStorage.m b/React/Modules/RCTAsyncLocalStorage.m
@@ -96,6 +96,26 @@ static void RCTMergeRecursive(NSMutableDictionary *destination, NSDictionary *so
   }
 }
 
+static dispatch_queue_t RCTGetMethodQueue()
+{
+  // We want all instances to share the same queue since they will be reading/writing the same files.
+  static dispatch_queue_t queue;
+  static dispatch_once_t onceToken;
+  dispatch_once(&onceToken, ^{
+    queue = dispatch_queue_create("com.facebook.React.AsyncLocalStorageQueue", DISPATCH_QUEUE_SERIAL);
+  });
+  return queue;
+}
+
+static BOOL RCTHasCreatedStorageDirectory = NO;
+static NSError *RCTDeleteStorageDirectory()
+{
+  NSError *error;
+  [[NSFileManager defaultManager] removeItemAtPath:RCTGetStorageDir() error:&error];
+  RCTHasCreatedStorageDirectory = NO;
+  return error;
+}
+
 #pragma mark - RCTAsyncLocalStorage
 
 @implementation RCTAsyncLocalStorage
@@ -113,26 +133,20 @@ @implementation RCTAsyncLocalStorage
 
 - (dispatch_queue_t)methodQueue
 {
-  // We want all instances to share the same queue since they will be reading/writing the same files.
-  static dispatch_queue_t queue;
-  static dispatch_once_t onceToken;
-  dispatch_once(&onceToken, ^{
-    queue = dispatch_queue_create("com.facebook.React.AsyncLocalStorageQueue", DISPATCH_QUEUE_SERIAL);
-  });
-  return queue;
+  return RCTGetMethodQueue();
 }
 
-+ (NSError *)clearAllData
++ (void)clearAllData
 {
-  NSError *error;
-  [[NSFileManager defaultManager] removeItemAtPath:RCTGetStorageDir() error:&error];
-  return error;
+  dispatch_async(RCTGetMethodQueue(), ^{
+    RCTDeleteStorageDirectory();
+  });
 }
 
 - (void)invalidate
 {
   if (_clearOnInvalidate) {
-    [RCTAsyncLocalStorage clearAllData];
+    RCTDeleteStorageDirectory();
   }
   _clearOnInvalidate = NO;
   _manifest = [[NSMutableDictionary alloc] init];
@@ -156,27 +170,31 @@ - (NSString *)_filePathForKey:(NSString *)key
 
 - (id)_ensureSetup
 {
-  if (_haveSetup) {
-    return nil;
-  }
-  _storageDirectory = RCTGetStorageDir();
-  NSError *error;
-  [[NSFileManager defaultManager] createDirectoryAtPath:_storageDirectory
-                            withIntermediateDirectories:YES
-                                             attributes:nil
-                                                  error:&error];
-  if (error) {
-    return RCTMakeError(@"Failed to create storage directory.", error, nil);
+  RCTAssertThread(RCTGetMethodQueue(), @"Must be executed on storage thread");
+
+  NSError *error = nil;
+  if (!RCTHasCreatedStorageDirectory) {
+    _storageDirectory = RCTGetStorageDir();
+    [[NSFileManager defaultManager] createDirectoryAtPath:_storageDirectory
+                              withIntermediateDirectories:YES
+                                               attributes:nil
+                                                    error:&error];
+    if (error) {
+      return RCTMakeError(@"Failed to create storage directory.", error, nil);
+    }
+    RCTHasCreatedStorageDirectory = YES;
   }
-  _manifestPath = [_storageDirectory stringByAppendingPathComponent:kManifestFilename];
-  NSDictionary *errorOut;
-  NSString *serialized = RCTReadFile(_manifestPath, nil, &errorOut);
-  _manifest = serialized ? [RCTJSONParse(serialized, &error) mutableCopy] : [[NSMutableDictionary alloc] init];
-  if (error) {
-    RCTLogWarn(@"Failed to parse manifest - creating new one.\n\n%@", error);
-    _manifest = [[NSMutableDictionary alloc] init];
+  if (!_haveSetup) {
+    _manifestPath = [_storageDirectory stringByAppendingPathComponent:kManifestFilename];
+    NSDictionary *errorOut;
+    NSString *serialized = RCTReadFile(_manifestPath, nil, &errorOut);
+    _manifest = serialized ? [RCTJSONParse(serialized, &error) mutableCopy] : [[NSMutableDictionary alloc] init];
+    if (error) {
+      RCTLogWarn(@"Failed to parse manifest - creating new one.\n\n%@", error);
+      _manifest = [[NSMutableDictionary alloc] init];
+    }
+    _haveSetup = YES;
   }
-  _haveSetup = YES;
   return nil;
 }
 
@@ -349,7 +367,7 @@ - (id)_writeEntry:(NSArray *)entry
 RCT_EXPORT_METHOD(clear:(RCTResponseSenderBlock)callback)
 {
   _manifest = [[NSMutableDictionary alloc] init];
-  NSError *error = [RCTAsyncLocalStorage clearAllData];
+  NSError *error = RCTDeleteStorageDirectory();
   if (callback) {
     callback(@[RCTNullIfNil(error)]);
   }