Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-38035

Memory and file descriptor leak in QFontCache

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: P1: Critical
    • Resolution: Done
    • Affects Version/s: 4.8.2
    • Fix Version/s: 4.8.x
    • Component/s: GUI: Font handling
    • Labels:
      None
    • Commits:
      I27bea376f4ec0888463b4ec3ed1a6bef00d041f8

      Description

      Our application uses fonts across multiple threads (some threads which only exist temporarily) - and we have had several crashes due to "Failed to start thread - too many open files"...

      I traced this down to a file descriptor leak, because QFontCache (there is one per thread) does not properly clean up its font engines, and font data engines. These leak memory and file descriptors.

      A quick google search find some other reports of memory leaks:
      http://qt-project.org/forums/viewthread/21587
      https://bugreports.qt-project.org/browse/QTBUG-12562

      Here is a simple test program for you to recreate the leak:

      int main(int argc, char* argv[])
      {
          class tFontLeak : public QThread
          {
          public:
              tFontLeak() : QThread() {}
              void run()
              {
                  QFont font = QApplication::font();
                  QFontMetrics( font ).width( "test" );
              }
          };
      
          while( 1 )
          {
              tFontLeak test;
              test.start();
              test.wait();
          }
      }
      

      I have spent a few days trying to fix the problem. I have come up with a fix - which fixes the leak when thread/QFontCache is destroyed - but have not tested (for example) the QFontCache timer still cleans up the cache properly. We need a fix, but are not confident enough with this fix yet - we need you guys with more knowledge on the whole font caching design - to review and approve it (or come up with your own fix).

      Looking at the code

      {qfont.cpp; qfontengine.cpp; qrawfont.cpp; qrawfon_win.cpp; qstatictext.cpp; qtextengine.cpp}

      • There is no consistency over who deletes the font engines. Some font engine users deref without deleting.
      • Some delete, but only if cache_count == 0 (i.e. cache logic is spread throughout the code).

      Each font engine in the cache is reference counted. So in theory this should be simple. every user of a font engine shall increment the ref count, and when finished decrement the reference count and delete the font engine if 0.

      The main problem is that QFontCache itself does not increment the ref count - meaning if the QFontCache is deleted before all the font engine references, the font engines will leak.

      The fix seems fairly straightforward (see the attached patch):

      • Make the cache also use the ref counts
      • Make everyone who decrements a ref count check for 0 and delete
      • Move all cache logic to the cache.

        Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

            Assignee:
            siyuan.navico Simon Yuan
            Reporter:
            siyuan.navico Simon Yuan
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Gerrit Reviews

                There are no open Gerrit changes