Details
-
Bug
-
Resolution: Fixed
-
P2: Important
-
5.15, 6.2, 6.5, 6.6, 6.7
-
None
-
Arch Linux; Linux 6.6; Mixxx 2.4-beta; Pipewire 1.0.0
-
-
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
- resulted from
-
QTBUG-100431 Crash in libQt5Qml V4 engine caused by wrong memory access
- Closed
For Gerrit Dashboard: QTBUG-120450 | ||||||
---|---|---|---|---|---|---|
# | Subject | Branch | Project | Status | CR | V |
527948,5 | masm: Don't crash on failed MADV_DONTNEED on Linux | dev | qt/qtdeclarative | Status: MERGED | +2 | 0 |
535853,2 | masm: Don't crash on failed MADV_DONTNEED on Linux | 6.7 | qt/qtdeclarative | Status: MERGED | +2 | 0 |
535989,2 | masm: Don't crash on failed MADV_DONTNEED on Linux | 6.6 | qt/qtdeclarative | Status: MERGED | +2 | 0 |
536280,2 | masm: Don't crash on failed MADV_DONTNEED on Linux | tqtc/lts-6.5 | qt/tqtc-qtdeclarative | Status: MERGED | +2 | 0 |
536346,2 | masm: Don't crash on failed MADV_DONTNEED on Linux | tqtc/lts-6.2 | qt/tqtc-qtdeclarative | Status: MERGED | +2 | 0 |
537968,2 | masm: Don't crash on failed MADV_DONTNEED on Linux | tqtc/lts-5.15 | qt/tqtc-qtdeclarative | Status: MERGED | +2 | 0 |