Kaynağa Gözat

Fix accessing a destroyed object (#3721) (#3725)

This was caused by running under Asan with XCode 11. Apparently, previous versions of XCode cannot catch it (which is why Travis is passing, apparently).

Also:
* enable "Detect use of stack after return" option for Asan. AFAIU, it can only be enabled in the scheme. It's already enabled for unit tests, this PR additionally enables it for integration tests;
* make Asan failures fail Travis build. They were originally not considered blocking due to some sporadic failures. I haven't seen those for some time, and worst case scenario, we can revert this change.
Konstantin Varlamov 6 yıl önce
ebeveyn
işleme
5dc46ad17d

+ 0 - 4
.travis.yml

@@ -447,12 +447,8 @@ jobs:
     # need to make them fatal for the purposes of the test run.
 
   # TODO(varconst): disallow sanitizers to fail once we fix all existing issues.
-    - env:
-      - PROJECT=Firestore PLATFORM=macOS METHOD=cmake SANITIZERS=asan
     - env:
       - PROJECT=Firestore PLATFORM=macOS METHOD=cmake SANITIZERS=tsan
-    - env:
-      - PROJECT=Firestore PLATFORM=iOS METHOD=xcodebuild SANITIZERS=asan
     - env:
       - PROJECT=Firestore PLATFORM=iOS METHOD=xcodebuild SANITIZERS=tsan
     - env:

+ 11 - 14
Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_IntegrationTests_iOS.xcscheme

@@ -26,7 +26,17 @@
       buildConfiguration = "Debug"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
-      shouldUseLaunchSchemeArgsEnv = "YES">
+      shouldUseLaunchSchemeArgsEnv = "YES"
+      enableASanStackUseAfterReturn = "YES">
+      <MacroExpansion>
+         <BuildableReference
+            BuildableIdentifier = "primary"
+            BlueprintIdentifier = "DE03B2941F2149D600A30B9C"
+            BuildableName = "Firestore_IntegrationTests_iOS.xctest"
+            BlueprintName = "Firestore_IntegrationTests_iOS"
+            ReferencedContainer = "container:Firestore.xcodeproj">
+         </BuildableReference>
+      </MacroExpansion>
       <Testables>
          <TestableReference
             skipped = "NO">
@@ -39,17 +49,6 @@
             </BuildableReference>
          </TestableReference>
       </Testables>
-      <MacroExpansion>
-         <BuildableReference
-            BuildableIdentifier = "primary"
-            BlueprintIdentifier = "DE03B2941F2149D600A30B9C"
-            BuildableName = "Firestore_IntegrationTests_iOS.xctest"
-            BlueprintName = "Firestore_IntegrationTests_iOS"
-            ReferencedContainer = "container:Firestore.xcodeproj">
-         </BuildableReference>
-      </MacroExpansion>
-      <AdditionalOptions>
-      </AdditionalOptions>
    </TestAction>
    <LaunchAction
       buildConfiguration = "Debug"
@@ -70,8 +69,6 @@
             ReferencedContainer = "container:Firestore.xcodeproj">
          </BuildableReference>
       </MacroExpansion>
-      <AdditionalOptions>
-      </AdditionalOptions>
    </LaunchAction>
    <ProfileAction
       buildConfiguration = "Release"

+ 2 - 1
Firestore/core/src/firebase/firestore/core/view.cc

@@ -212,7 +212,8 @@ ViewDocumentChanges View::ComputeDocumentChanges(
   if (limit != Query::kNoLimit &&
       new_document_set.size() > static_cast<size_t>(limit)) {
     for (size_t i = new_document_set.size() - limit; i > 0; --i) {
-      const Document& old_doc = *new_document_set.GetLastDocument();
+      absl::optional<Document> found = new_document_set.GetLastDocument();
+      const Document& old_doc = *found;
       new_document_set = new_document_set.erase(old_doc.key());
       new_mutated_keys = new_mutated_keys.erase(old_doc.key());
       change_set.AddChange(