From 0b76d6a28e6978151bf245a775329cdae5e574d5 Mon Sep 17 00:00:00 2001
From: Narayan Kamath <narayan@google.com>
Date: Wed, 7 Sep 2016 13:14:40 +0100
Subject: [PATCH] fd_utils: Fix broken usage of iterators.

There were two separate issues here :
- RestatInternal was using an iterator after a call to erase(). This
  will not work because it will be invalidated.
- The "standard" for loop idiom for iterating over a map while making
  structural changes to it is broken. Switch to a while loop and treat
  cases where elements are erased differently from cases where they
  aren't.

bug: 31092930
bug: 30963384
Change-Id: I261d59239558230dd8cdd1d1cb5b9e2448a4c23f
---
 core/jni/fd_utils-inl.h | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/core/jni/fd_utils-inl.h b/core/jni/fd_utils-inl.h
index 44338eac8b1f7..c67662bd8a88d 100644
--- a/core/jni/fd_utils-inl.h
+++ b/core/jni/fd_utils-inl.h
@@ -446,8 +446,8 @@ class FileDescriptorTable {
     //
     // (a) they continue to be open.
     // (b) they refer to the same file.
-    std::unordered_map<int, FileDescriptorInfo*>::iterator it;
-    for (it = open_fd_map_.begin(); it != open_fd_map_.end(); ++it) {
+    std::unordered_map<int, FileDescriptorInfo*>::iterator it = open_fd_map_.begin();
+    while (it != open_fd_map_.end()) {
       std::set<int>::const_iterator element = open_fds.find(it->first);
       if (element == open_fds.end()) {
         // The entry from the file descriptor table is no longer in the list
@@ -457,11 +457,10 @@ class FileDescriptorTable {
         // TODO(narayan): This will be an error in a future android release.
         // error = true;
         // ALOGW("Zygote closed file descriptor %d.", it->first);
-        open_fd_map_.erase(it);
+        it = open_fd_map_.erase(it);
       } else {
         // The entry from the file descriptor table is still open. Restat
         // it and check whether it refers to the same file.
-        open_fds.erase(element);
         const bool same_file = it->second->Restat();
         if (!same_file) {
           // The file descriptor refers to a different description. We must
@@ -473,11 +472,20 @@ class FileDescriptorTable {
             // We flag an error and remove it from the list of files we're
             // tracking.
             error = true;
-            open_fd_map_.erase(it);
+            it = open_fd_map_.erase(it);
+          } else {
+            // Successfully restatted the file, move on to the next open FD.
+            ++it;
           }
         } else {
-          // It's the same file. Nothing to do here.
+          // It's the same file. Nothing to do here. Move on to the next open
+          // FD.
+          ++it;
         }
+
+        // Finally, remove the FD from the set of open_fds. We do this last because
+        // |element| will not remain valid after a call to erase.
+        open_fds.erase(element);
       }
     }