0030-Memory-and-file-descriptor-leak-in-QFontCache.patch

   1 From 45693cc638d10890f2816a38d38de6ddaf04ffd3 Mon Sep 17 00:00:00 2001
   2 From: Simon Yuan <simon.yuan@navico.com>
   3 Date: Wed, 2 Apr 2014 16:02:04 +1300
   4 Subject: [PATCH 30/74] Memory and file descriptor leak in QFontCache
   5
   6 Make the cache also use the ref counts
   7 Make everyone who decrements a ref count check for 0 and delete
   8 Move all cache logic to the cache
   9 Same idea as 36cb3f3 and b3dae68 in Qt 5 without the extra stuff
  10
  11 Task-number: QTBUG-38035
  12 Change-Id: I27bea376f4ec0888463b4ec3ed1a6bef00d041f8
  13 Reviewed-by: Konstantin Ritt <ritt.ks@gmail.com>
  14 Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@digia.com>
  15 ---
  16  src/gui/text/qfont.cpp        | 102 +++++++++++++++++-------------------------
  17  src/gui/text/qfontengine.cpp  |   7 +--
  18  src/gui/text/qrawfont.cpp     |  13 +++---
  19  src/gui/text/qrawfont_win.cpp |   4 +-
  20  src/gui/text/qstatictext.cpp  |   6 +--
  21  src/gui/text/qtextengine.cpp  |   7 +--
  22  6 files changed, 55 insertions(+), 84 deletions(-)
  23
  24 diff --git a/src/gui/text/qfont.cpp b/src/gui/text/qfont.cpp
  25 index 7e94c1e..fa9bb70 100644
  26 --- a/src/gui/text/qfont.cpp
  27 +++ b/src/gui/text/qfont.cpp
  28 @@ -275,8 +275,8 @@ QFontPrivate::QFontPrivate(const QFontPrivate &other)
  29
  30  QFontPrivate::~QFontPrivate()
  31  {
  32 -    if (engineData)
  33 -        engineData->ref.deref();
  34 +    if (engineData && !engineData->ref.deref())
  35 +        delete engineData;
  36      engineData = 0;
  37      if (scFont && scFont != this)
  38          scFont->ref.deref();
  39 @@ -298,7 +298,8 @@ QFontEngine *QFontPrivate::engineForScript(int script) const
  40          script = QUnicodeTables::Common;
  41      if (engineData && engineData->fontCache != QFontCache::instance()) {
  42          // throw out engineData that came from a different thread
  43 -        engineData->ref.deref();
  44 +        if (!engineData->ref.deref())
  45 +            delete engineData;
  46          engineData = 0;
  47      }
  48      if (!engineData || !QT_FONT_ENGINE_FROM_DATA(engineData, script))
  49 @@ -417,13 +418,13 @@ QFontEngineData::~QFontEngineData()
  50  {
  51  #if !defined(Q_WS_MAC)
  52      for (int i = 0; i < QUnicodeTables::ScriptCount; ++i) {
  53 -        if (engines[i])
  54 -            engines[i]->ref.deref();
  55 +        if (engines[i] && !engines[i]->ref.deref())
  56 +            delete engines[i];
  57          engines[i] = 0;
  58      }
  59  #else
  60 -    if (engine)
  61 -        engine->ref.deref();
  62 +    if (engine && !engine->ref.deref())
  63 +        delete engine;
  64      engine = 0;
  65  #endif // Q_WS_X11 || Q_WS_WIN || Q_WS_MAC
  66  }
  67 @@ -770,8 +771,8 @@ QFont::QFont(QFontPrivate *data)
  68  void QFont::detach()
  69  {
  70      if (d->ref == 1) {
  71 -        if (d->engineData)
  72 -            d->engineData->ref.deref();
  73 +        if (d->engineData && !d->engineData->ref.deref())
  74 +            delete d->engineData;
  75          d->engineData = 0;
  76          if (d->scFont && d->scFont != d.data())
  77              d->scFont->ref.deref();
  78 @@ -2819,7 +2820,7 @@ QFontCache::~QFontCache()
  79          EngineDataCache::ConstIterator it = engineDataCache.constBegin(),
  80                                   end = engineDataCache.constEnd();
  81          while (it != end) {
  82 -            if (it.value()->ref == 0)
  83 +            if (it.value()->ref.deref() == 0)
  84                  delete it.value();
  85              else
  86                  FC_DEBUG("QFontCache::~QFontCache: engineData %p still has refcount %d",
  87 @@ -2827,24 +2828,6 @@ QFontCache::~QFontCache()
  88              ++it;
  89          }
  90      }
  91 -    EngineCache::ConstIterator it = engineCache.constBegin(),
  92 -                         end = engineCache.constEnd();
  93 -    while (it != end) {
  94 -        if (--it.value().data->cache_count == 0) {
  95 -            if (it.value().data->ref == 0) {
  96 -                FC_DEBUG("QFontCache::~QFontCache: deleting engine %p key=(%d / %g %g %d %d %d)",
  97 -                         it.value().data, it.key().script, it.key().def.pointSize,
  98 -                         it.key().def.pixelSize, it.key().def.weight, it.key().def.style,
  99 -                         it.key().def.fixedPitch);
 100 -
 101 -                delete it.value().data;
 102 -            } else {
 103 -                FC_DEBUG("QFontCache::~QFontCache: engine = %p still has refcount %d",
 104 -                         it.value().data, int(it.value().data->ref));
 105 -            }
 106 -        }
 107 -        ++it;
 108 -    }
 109  }
 110
 111  void QFontCache::clear()
 112 @@ -2856,16 +2839,14 @@ void QFontCache::clear()
 113              QFontEngineData *data = it.value();
 114  #if !defined(Q_WS_MAC)
 115              for (int i = 0; i < QUnicodeTables::ScriptCount; ++i) {
 116 -                if (data->engines[i]) {
 117 -                    data->engines[i]->ref.deref();
 118 -                    data->engines[i] = 0;
 119 -                }
 120 +                if (data->engines[i] && !data->engines[i]->ref.deref())
 121 +                    delete data->engines[i];
 122 +                data->engines[i] = 0;
 123              }
 124  #else
 125 -            if (data->engine) {
 126 -                data->engine->ref.deref();
 127 -                data->engine = 0;
 128 -            }
 129 +            if (data->engine && !data->engine->ref.deref())
 130 +                delete data->engine;
 131 +            data->engine = 0;
 132  #endif
 133              ++it;
 134          }
 135 @@ -2873,15 +2854,7 @@ void QFontCache::clear()
 136
 137      for (EngineCache::Iterator it = engineCache.begin(), end = engineCache.end();
 138           it != end; ++it) {
 139 -        if (it->data->ref == 0) {
 140 -            delete it->data;
 141 -            it->data = 0;
 142 -        }
 143 -    }
 144 -
 145 -    for (EngineCache::Iterator it = engineCache.begin(), end = engineCache.end();
 146 -         it != end; ++it) {
 147 -        if (it->data && it->data->ref == 0) {
 148 +        if (it->data->ref.deref() == 0) {
 149              delete it->data;
 150              it->data = 0;
 151          }
 152 @@ -2916,6 +2889,8 @@ void QFontCache::insertEngineData(const Key &key, QFontEngineData *engineData)
 153  {
 154      FC_DEBUG("QFontCache: inserting new engine data %p", engineData);
 155
 156 +    Q_ASSERT(!engineDataCache.contains(key));
 157 +    engineData->ref.ref(); // the cache has a reference
 158      engineDataCache.insert(key, engineData);
 159      increaseCost(sizeof(QFontEngineData));
 160  }
 161 @@ -2946,6 +2921,11 @@ void QFontCache::insertEngine(const Key &key, QFontEngine *engine)
 162      Engine data(engine);
 163      data.timestamp = ++current_timestamp;
 164
 165 +    QFontEngine *oldEngine = engineCache.value(key).data;
 166 +    engine->ref.ref(); // the cache has a reference
 167 +    if (oldEngine && !oldEngine->ref.deref())
 168 +        delete oldEngine;
 169 +
 170      engineCache.insert(key, data);
 171
 172      // only increase the cost if this is the first time we insert the engine
 173 @@ -3005,12 +2985,11 @@ void QFontCache::cleanupPrinterFonts()
 174                  continue;
 175              }
 176
 177 -            if(it.value()->ref != 0) {
 178 -                for(int i = 0; i < QUnicodeTables::ScriptCount; ++i) {
 179 -                    if(it.value()->engines[i]) {
 180 -                        it.value()->engines[i]->ref.deref();
 181 -                        it.value()->engines[i] = 0;
 182 -                    }
 183 +            if (it.value()->ref > 1) {
 184 +                for (int i = 0; i < QUnicodeTables::ScriptCount; ++i) {
 185 +                    if (it.value()->engines[i] && !it.value()->engines[i]->ref.deref())
 186 +                        delete it.value()->engines[i];
 187 +                    it.value()->engines[i] = 0;
 188                  }
 189                  ++it;
 190              } else {
 191 @@ -3021,7 +3000,8 @@ void QFontCache::cleanupPrinterFonts()
 192
 193                  FC_DEBUG("    %p", rem.value());
 194
 195 -                delete rem.value();
 196 +                if (!rem.value()->ref.deref())
 197 +                    delete rem.value();
 198                  engineDataCache.erase(rem);
 199              }
 200          }
 201 @@ -3030,7 +3010,7 @@ void QFontCache::cleanupPrinterFonts()
 202      EngineCache::Iterator it = engineCache.begin(),
 203                           end = engineCache.end();
 204      while(it != end) {
 205 -        if (it.value().data->ref != 0 || it.key().screen == 0) {
 206 +        if (it.value().data->ref != 1 || it.key().screen == 0) {
 207              ++it;
 208              continue;
 209          }
 210 @@ -3044,7 +3024,8 @@ void QFontCache::cleanupPrinterFonts()
 211              FC_DEBUG("    DELETE: last occurrence in cache");
 212
 213              decreaseCost(it.value().data->cache_cost);
 214 -            delete it.value().data;
 215 +            if (!it.value().data->ref.deref())
 216 +                delete it.value().data;
 217          }
 218
 219          engineCache.erase(it++);
 220 @@ -3093,7 +3074,7 @@ void QFontCache::timerEvent(QTimerEvent *)
 221  #  endif // Q_WS_X11 || Q_WS_WIN
 222  #endif // QFONTCACHE_DEBUG
 223
 224 -            if (it.value()->ref != 0)
 225 +            if (it.value()->ref > 1)
 226                  in_use_cost += engine_data_cost;
 227          }
 228      }
 229 @@ -3109,7 +3090,7 @@ void QFontCache::timerEvent(QTimerEvent *)
 230                       int(it.value().data->ref), it.value().data->cache_count,
 231                       it.value().data->cache_cost);
 232
 233 -            if (it.value().data->ref != 0)
 234 +            if (it.value().data->ref > 1)
 235                  in_use_cost += it.value().data->cache_cost / it.value().data->cache_count;
 236          }
 237
 238 @@ -3159,7 +3140,7 @@ void QFontCache::timerEvent(QTimerEvent *)
 239          EngineDataCache::Iterator it = engineDataCache.begin(),
 240                                   end = engineDataCache.end();
 241          while (it != end) {
 242 -            if (it.value()->ref != 0) {
 243 +            if (it.value()->ref > 1) {
 244                  ++it;
 245                  continue;
 246              }
 247 @@ -3187,7 +3168,7 @@ void QFontCache::timerEvent(QTimerEvent *)
 248          uint least_popular = ~0u;
 249
 250          for (; it != end; ++it) {
 251 -            if (it.value().data->ref != 0)
 252 +            if (it.value().data->ref > 1)
 253                  continue;
 254
 255              if (it.value().timestamp < oldest &&
 256 @@ -3200,7 +3181,7 @@ void QFontCache::timerEvent(QTimerEvent *)
 257          FC_DEBUG("    oldest %u least popular %u", oldest, least_popular);
 258
 259          for (it = engineCache.begin(); it != end; ++it) {
 260 -            if (it.value().data->ref == 0 &&
 261 +            if (it.value().data->ref == 1 &&
 262                   it.value().timestamp == oldest &&
 263                   it.value().hits == least_popular)
 264                  break;
 265 @@ -3216,7 +3197,8 @@ void QFontCache::timerEvent(QTimerEvent *)
 266                  FC_DEBUG("    DELETE: last occurrence in cache");
 267
 268                  decreaseCost(it.value().data->cache_cost);
 269 -                delete it.value().data;
 270 +                if (!it.value().data->ref.deref())
 271 +                    delete it.value().data;
 272              } else {
 273                  /*
 274                    this particular font engine is in the cache multiple
 275 diff --git a/src/gui/text/qfontengine.cpp b/src/gui/text/qfontengine.cpp
 276 index 9de475c..bf108c4 100644
 277 --- a/src/gui/text/qfontengine.cpp
 278 +++ b/src/gui/text/qfontengine.cpp
 279 @@ -1325,11 +1325,8 @@ QFontEngineMulti::~QFontEngineMulti()
 280  {
 281      for (int i = 0; i < engines.size(); ++i) {
 282          QFontEngine *fontEngine = engines.at(i);
 283 -        if (fontEngine) {
 284 -            fontEngine->ref.deref();
 285 -            if (fontEngine->cache_count == 0 && fontEngine->ref == 0)
 286 -                delete fontEngine;
 287 -        }
 288 +        if (fontEngine && !fontEngine->ref.deref())
 289 +            delete fontEngine;
 290      }
 291  }
 292
 293 diff --git a/src/gui/text/qrawfont.cpp b/src/gui/text/qrawfont.cpp
 294 index 2b7554a..cb2bcb3 100644
 295 --- a/src/gui/text/qrawfont.cpp
 296 +++ b/src/gui/text/qrawfont.cpp
 297 @@ -682,8 +682,7 @@ void QRawFont::setPixelSize(qreal pixelSize)
 298      if (d->fontEngine != 0)
 299          d->fontEngine->ref.ref();
 300
 301 -    oldFontEngine->ref.deref();
 302 -    if (oldFontEngine->cache_count == 0 && oldFontEngine->ref == 0)
 303 +    if (!oldFontEngine->ref.deref())
 304          delete oldFontEngine;
 305  }
 306
 307 @@ -693,12 +692,10 @@ void QRawFont::setPixelSize(qreal pixelSize)
 308  void QRawFontPrivate::cleanUp()
 309  {
 310      platformCleanUp();
 311 -    if (fontEngine != 0) {
 312 -        fontEngine->ref.deref();
 313 -        if (fontEngine->cache_count == 0 && fontEngine->ref == 0)
 314 -            delete fontEngine;
 315 -        fontEngine = 0;
 316 -    }
 317 +    if (fontEngine != 0 && !fontEngine->ref.deref())
 318 +        delete fontEngine;
 319 +    fontEngine = 0;
 320 +
 321      hintingPreference = QFont::PreferDefaultHinting;
 322  }
 323
 324 diff --git a/src/gui/text/qrawfont_win.cpp b/src/gui/text/qrawfont_win.cpp
 325 index 6923aae..9b66886 100644
 326 --- a/src/gui/text/qrawfont_win.cpp
 327 +++ b/src/gui/text/qrawfont_win.cpp
 328 @@ -600,11 +600,11 @@ void QRawFontPrivate::platformLoadFromData(const QByteArray &fontData,
 329              if (request.family != fontEngine->fontDef.family) {
 330                  qWarning("QRawFont::platformLoadFromData: Failed to load font. "
 331                           "Got fallback instead: %s", qPrintable(fontEngine->fontDef.family));
 332 -                if (fontEngine->cache_count == 0 && fontEngine->ref == 0)
 333 +                if (fontEngine->ref == 0)
 334                      delete fontEngine;
 335                  fontEngine = 0;
 336              } else {
 337 -                Q_ASSERT(fontEngine->cache_count == 0 && fontEngine->ref == 0);
 338 +                Q_ASSERT(fontEngine->ref == 0);
 339
 340                  // Override the generated font name
 341                  static_cast<QFontEngineWin *>(fontEngine)->uniqueFamilyName = uniqueFamilyName;
 342 diff --git a/src/gui/text/qstatictext.cpp b/src/gui/text/qstatictext.cpp
 343 index 657da33..b111200 100644
 344 --- a/src/gui/text/qstatictext.cpp
 345 +++ b/src/gui/text/qstatictext.cpp
 346 @@ -724,10 +724,8 @@ QStaticTextItem::~QStaticTextItem()
 347
 348  void QStaticTextItem::setFontEngine(QFontEngine *fe)
 349  {
 350 -    if (m_fontEngine != 0) {
 351 -        if (!m_fontEngine->ref.deref())
 352 -            delete m_fontEngine;
 353 -    }
 354 +    if (m_fontEngine != 0 && !m_fontEngine->ref.deref())
 355 +        delete m_fontEngine;
 356
 357      m_fontEngine = fe;
 358      if (m_fontEngine != 0)
 359 diff --git a/src/gui/text/qtextengine.cpp b/src/gui/text/qtextengine.cpp
 360 index b371237..f4b86b0 100644
 361 --- a/src/gui/text/qtextengine.cpp
 362 +++ b/src/gui/text/qtextengine.cpp
 363 @@ -1453,11 +1453,8 @@ void QTextEngine::shape(int item) const
 364
 365  static inline void releaseCachedFontEngine(QFontEngine *fontEngine)
 366  {
 367 -    if (fontEngine) {
 368 -        fontEngine->ref.deref();
 369 -        if (fontEngine->cache_count == 0 && fontEngine->ref == 0)
 370 -            delete fontEngine;
 371 -    }
 372 +    if (fontEngine && !fontEngine->ref.deref())
 373 +        delete fontEngine;
 374  }
 375
 376  void QTextEngine::resetFontEngineCache()
 377 --
 378 1.9.3
 379