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

Allocating or deallocating a QJSEngine object causes a crash if the application has called mlockall(MCL_CURRENT|MCL_FUTURE)

    XMLWordPrintable

Details

    • Linux/Wayland, Linux/X11, Linux/Yocto, Linux/Other display system
    • 524d260c5 (dev), 1ead022df (6.7), a2e006131 (6.6), 5c42f57e8 (tqtc/lts-6.5), cb8ecdbf4 (tqtc/lts-6.2), f1d0468a0 (tqtc/lts-5.15)

    Description

      Description

      The current beta version (2.4) of Mixxx crashes when allocating a QJSEngine if using Pipewire with the mem.mlock-all option set in the configuration.

      With the above option enabled, the Pipewire library calls mlockall(MCL_CURRENT|MCL_FUTURE) so the pages mapped by the audio application aren't swapped out to minimize audio latency.
      Then, upon QJSEngine object creation, WTF::OSAllocator::reserveUncommitted is called and the following code from qtdeclarative-6.6/src/3rdparty/masm/wtf/OSAllocatorPosix.cpp#117 is executed:

          while (madvise(result, bytes, MADV_DONTNEED)) {
              if (errno != EAGAIN)
                  CRASH();
          }
      

      This madvise call fails with errno = EINVAL because the region is locked and so the application crashes. The reason of the failure is found in the man page for madvise:

      SYNOPSIS
             #include <sys/mman.h>
      
             int madvise(void addr[.length], size_t length, int advice);
      
      ERRORS
             EINVAL advice is MADV_DONTNEED or MADV_REMOVE and the specified address range in‐
                    cludes locked, Huge TLB pages, or VM_PFNMAP pages.
      

      Note that the application would also crash upon the object destruction because of qtdeclarative-6.6/src/3rdparty/masm/wtf/OSAllocatorPosix.cpp#252 which does the same thing.

      Demonstration

      Here's a minimal example to reproduce the bug:

      #include <sys/mman.h>
      
      #include <QCoreApplication>
      #include <QDebug>
      #include <QJSEngine>
      
      int main(int argc, char *argv[])
      {
          QCoreApplication a(argc, argv);
      
          qDebug() << QT_VERSION_MAJOR << QT_VERSION_MINOR << QT_VERSION_PATCH;
      
          // lock memory (as done by for example Pipewire with a typical pro-audio configuration).
          mlockall(MCL_CURRENT|MCL_FUTURE);
      
          QJSEngine engine;
          // no need to do anything else, we've crashed by now.
          // if we could get past this point, the object destruction would also crash.
      }

      Proposed solution

      I suggest the following patch to address this issue:

      diff --git a/src/3rdparty/masm/wtf/OSAllocatorPosix.cpp b/src/3rdparty/masm/wtf/OSAllocatorPosix.cpp
      index a8990a92b4..484f6c1ca5 100644
      --- a/src/3rdparty/masm/wtf/OSAllocatorPosix.cpp
      +++ b/src/3rdparty/masm/wtf/OSAllocatorPosix.cpp
      @@ -37,6 +37,9 @@
       #include <wtf/UnusedParam.h>
      
       #if OS(LINUX)
      +
      +#include <cstdio>
      +
       #include <sys/syscall.h>
       #ifndef MFD_CLOEXEC
       #define MFD_CLOEXEC         0x0001U
      @@ -114,11 +117,6 @@ void* OSAllocator::reserveUncommitted(size_t bytes, Usage usage, bool writable,
           if (result == MAP_FAILED)
               CRASH();
      
      -    while (madvise(result, bytes, MADV_DONTNEED)) {
      -        if (errno != EAGAIN)
      -            CRASH();
      -    }
      -
           if (fd != -1)
               close(fd);
       #else
      @@ -250,8 +248,10 @@ void OSAllocator::decommit(void* address, size_t bytes)
           mmap(address, bytes, PROT_NONE, MAP_FIXED | MAP_LAZY | MAP_PRIVATE | MAP_ANON, -1, 0);
       #elif OS(LINUX)
           while (madvise(address, bytes, MADV_DONTNEED)) {
      -        if (errno != EAGAIN)
      -            CRASH();
      +        if (errno != EAGAIN) {
      +            perror("wtf::OSAllocator::decommit: madvise");
      +            break;
      +        }
           }
           if (mprotect(address, bytes, PROT_NONE))
               CRASH();
      

      The madvise call can be completely removed from OSAllocator::reserveUncommitted because mmap already returns an uncommited and zero-filled mapping (and in the case of a memfd backing, ftruncate zero-fills it anyway). As for OSAllocator::decommit, while the madvise call is useful to decommit the mapping, it shouldn't cause a crash if it fails, especially when the reason is that the memory is locked to increase performance.

      EDIT: Updated patch and its explanation w.r.t. memfd (as it is used here instead of an anonymous mapping).

      Other information

      The bug was introduced in qt-declarative with change I9819f8d82a251e222b0b500991584d40e641b672.

      Also see Mixxx#12001 on Github.

      Attachments

        Issue Links

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

          Activity

            People

              qtqmlteam Qt Qml Team User
              napaalm Antonio Napolitano
              Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews