2010-11-10 Andreas Kling <kling@webkit.org>
authorandreas.kling@nokia.com <andreas.kling@nokia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Nov 2010 10:46:36 +0000 (10:46 +0000)
committerandreas.kling@nokia.com <andreas.kling@nokia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Nov 2010 10:46:36 +0000 (10:46 +0000)
        Reviewed by Kenneth Rohde Christiansen.

        [Qt][WK2] Fix re-use of memory-mapped files
        https://bugs.webkit.org/show_bug.cgi?id=49310

        QTemporaryFile::fileName() returns an empty string after close()
        so we have to keep separate track of the filename.

        Also, we can't reopen the file if it's been QFile::remove()d,
        so we defer unlinking until the MappedMemoryPool is destroyed or
        the CrashHandler kicks in.

        This makes re-use of memory-mapped files work (after we kill an
        assertion that the file size == the new mmap size - it's fine if
        the file is larger, too.)

        * Platform/qt/MappedMemoryPool.cpp:
        (WebKit::MappedMemoryPool::~MappedMemoryPool):
        (WebKit::MappedMemoryPool::clear):
        (WebKit::MappedMemoryPool::mapMemory):
        (WebKit::MappedMemoryPool::mapFile):
        * Platform/qt/MappedMemoryPool.h:
        (WebKit::MappedMemory::mappedFileName):
        * Shared/qt/CrashHandler.cpp:
        (WebKit::CrashHandler::deleteObjects):
        * Shared/qt/UpdateChunk.cpp:
        (WebKit::UpdateChunk::encode):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@71729 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKit2/ChangeLog
WebKit2/Platform/qt/MappedMemoryPool.cpp
WebKit2/Platform/qt/MappedMemoryPool.h
WebKit2/Shared/qt/CrashHandler.cpp
WebKit2/Shared/qt/UpdateChunk.cpp

index e04cbb0..41588cb 100644 (file)
@@ -1,3 +1,33 @@
+2010-11-10  Andreas Kling  <kling@webkit.org>
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        [Qt][WK2] Fix re-use of memory-mapped files
+        https://bugs.webkit.org/show_bug.cgi?id=49310
+
+        QTemporaryFile::fileName() returns an empty string after close()
+        so we have to keep separate track of the filename.
+
+        Also, we can't reopen the file if it's been QFile::remove()d,
+        so we defer unlinking until the MappedMemoryPool is destroyed or
+        the CrashHandler kicks in.
+
+        This makes re-use of memory-mapped files work (after we kill an
+        assertion that the file size == the new mmap size - it's fine if
+        the file is larger, too.)
+
+        * Platform/qt/MappedMemoryPool.cpp:
+        (WebKit::MappedMemoryPool::~MappedMemoryPool):
+        (WebKit::MappedMemoryPool::clear):
+        (WebKit::MappedMemoryPool::mapMemory):
+        (WebKit::MappedMemoryPool::mapFile):
+        * Platform/qt/MappedMemoryPool.h:
+        (WebKit::MappedMemory::mappedFileName):
+        * Shared/qt/CrashHandler.cpp:
+        (WebKit::CrashHandler::deleteObjects):
+        * Shared/qt/UpdateChunk.cpp:
+        (WebKit::UpdateChunk::encode):
+
 2010-11-09  Brady Eidson  <beidson@apple.com>
 
         Reviewed by the ever-picky Windows build-bot.
index 641312c..1b0f70d 100644 (file)
 
 namespace WebKit {
 
+MappedMemoryPool::~MappedMemoryPool()
+{
+    clear();
+}
+
+void MappedMemoryPool::clear()
+{
+    for (unsigned n = 0; n < m_pool.size(); ++n) {
+        MappedMemory& current = m_pool.at(n);
+        if (!current.file)
+            continue;
+        current.file->remove();
+        delete current.file;
+    }
+    m_pool.clear();
+}
+
 MappedMemory* MappedMemoryPool::mapMemory(size_t size)
 {
     for (unsigned n = 0; n < m_pool.size(); ++n) {
@@ -48,6 +65,7 @@ MappedMemory* MappedMemoryPool::mapMemory(size_t size)
     newMap.dataSize = size;
     newMap.file = new QTemporaryFile(QDir::tempPath() + "/WebKit2UpdateChunk");
     newMap.file->open(QIODevice::ReadWrite);
+    newMap.fileName = newMap.file->fileName();
     newMap.file->resize(newMap.mapSize());
     newMap.mappedBytes = newMap.file->map(0, newMap.mapSize());
     newMap.file->close();
@@ -60,7 +78,7 @@ MappedMemory* MappedMemoryPool::mapFile(QString fileName, size_t size)
 {
     for (unsigned n = 0; n < m_pool.size(); ++n) {
         MappedMemory& current = m_pool.at(n);
-        if (current.file->fileName() == fileName) {
+        if (current.fileName == fileName) {
             ASSERT(!current.isFree());
             return &current;
         }
@@ -70,13 +88,12 @@ MappedMemory* MappedMemoryPool::mapFile(QString fileName, size_t size)
     newMap.file = new QFile(fileName);
     newMap.dataSize = size;
     ASSERT(newMap.file->exists());
-    ASSERT(newMap.file->size() == newMap.mapSize());
+    ASSERT(newMap.file->size() >= newMap.mapSize());
     newMap.file->open(QIODevice::ReadWrite);
     newMap.mappedBytes = newMap.file->map(0, newMap.mapSize());
     ASSERT(newMap.mappedBytes);
     ASSERT(!newMap.isFree());
     newMap.file->close();
-    newMap.file->remove(); // The map stays alive even when the file is unlinked.
     m_pool.append(newMap);
     return &m_pool.last();
 }
index 530e7d0..4b3413b 100644 (file)
@@ -38,9 +38,11 @@ class MappedMemoryPool;
 
 struct MappedMemory {
 
-    QFile* mappedFile() const
+    QString mappedFileName() const
     {
-        return file;
+        ASSERT(file);
+        ASSERT(mappedBytes);
+        return fileName;
     }
 
     void markFree()
@@ -76,6 +78,7 @@ private:
     };
 
     QFile* file;
+    QString fileName;
     union {
         uchar* mappedBytes;
         Data* dataPtr;
@@ -94,8 +97,11 @@ public:
     MappedMemory* mapMemory(size_t size);
     MappedMemory* mapFile(QString fileName, size_t size);
 
+    void clear();
+
 private:
     MappedMemoryPool() { };
+    ~MappedMemoryPool();
 
     Vector<MappedMemory> m_pool;
 };
index 78ac713..9eb7d3d 100644 (file)
@@ -25,6 +25,7 @@
 
 #include "CrashHandler.h"
 
+#include "MappedMemoryPool.h"
 #include <csignal>
 #include <cstdlib>
 #include <wtf/AlwaysInline.h>
@@ -56,6 +57,8 @@ void CrashHandler::deleteObjects()
 {
     m_inDeleteObjects = true;
     qDeleteAll(m_objects);
+
+    MappedMemoryPool::instance()->clear();
 }
 
 } // namespace WebKit
index 60e5643..770c069 100644 (file)
@@ -62,7 +62,7 @@ UpdateChunk::~UpdateChunk()
 void UpdateChunk::encode(CoreIPC::ArgumentEncoder* encoder) const
 {
     encoder->encode(m_rect);
-    encoder->encode(String(m_mappedMemory->mappedFile()->fileName()));
+    encoder->encode(String(m_mappedMemory->mappedFileName()));
 
     m_mappedMemory = 0;
 }