Guard against double deletion of page views.

fpdfsdk/cpdfsdk_document.cpp

 // Copyright 2016 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "fpdfsdk/cpdfsdk_document.h"
 
 #include "core/fpdfapi/fpdf_parser/cpdf_array.h"
 #include "core/fpdfapi/fpdf_parser/cpdf_dictionary.h"
@@ skipping 117 matching lines @@
   }
   return m_pOccontent.get();
 }
 
 void CPDFSDK_Document::RemovePageView(UnderlyingPageType* pUnderlyingPage) {
   auto it = m_pageMap.find(pUnderlyingPage);
   if (it == m_pageMap.end())
     return;
 
   CPDFSDK_PageView* pPageView = it->second;
-  if (pPageView->IsLocked())
+  if (pPageView->IsLocked() || pPageView->IsBeingDestroyed())
     return;
 
+  // Mark the page view so we do not come into |RemovePageView| a second
+  // time while we're in the process of removing.
+  pPageView->SetBeingDestroyed();

[Tom Sepez, 2016/10/03 18:37:57]

Can we this to the dtor? Then the dtor checks for this and bails if set.

[dsinclair, 2016/10/03 18:47:08]

No, because we have to guard the map erasing and the killing of annotations. In
this specific bug, we get the iterator, get the KillFocusAnnotation, then we end
up back in here, get the iterator again and call erase(it). We then pop out the
stack and call erase(it) a second time, and the map doesn't like it when you
erase the same iterator value twice.

Maybe Destroyed was a bad name, IsBeingRemoved?

+
   // This must happen before we remove |pPageView| from the map because
   // |KillFocusAnnotIfNeeded| can call into the |GetPage| method which will
   // look for this page view in the map, if it doesn't find it a new one will
   // be created. We then have two page views pointing to the same page and
   // bad things happen.
   pPageView->KillFocusAnnotIfNeeded();
 
   // Remove the page from the map to make sure we don't accidentally attempt
   // to use the |pPageView| while we're cleaning it up.
   m_pageMap.erase(it);
@@ skipping 102 matching lines @@
   return GetPDFDocument()->GetUserPermissions() & nFlag;
 }
 
 IJS_Runtime* CPDFSDK_Document::GetJsRuntime() {
   return m_pEnv->GetJSRuntime();
 }
 
 CFX_WideString CPDFSDK_Document::GetPath() {
   return m_pEnv->JS_docGetFilePath();
 }