qiurui
/
qtbase
mirror of https://github.com/qt/qtbase.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Make QRandomAccessAsyncFile::open() an async operation 

The tricky case now would be to handle the case when the user tries
to call open() several times. Make sure that after the first operation
is started, all the subsequent attempts to open will fail until the
first operation is finished. Obviously, if the first open() was
successful, the file needs to be closed before it can be re-opened.
Add unit-tests to cover these cases.

Also add some tests for close() corner cases. These tests are useful
for this commit, as the logic of opening the file got more complicated.

Task-number: QTBUG-136763
Change-Id: Id505e6278fbc1d523a52b54e69ba896a3d754762
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
This commit is contained in:
Ivan Solovev 2025-09-09 15:30:20 +02:00
parent 7d7eec6697
commit 0f41ca136b
6 changed files with 203 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/corelib/io/qiooperation_p.h
View File

@ -37,6 +37,7 @@ public:
Read,
Write,
Flush,
Open,
Aborted,
};
Q_ENUM(Error)
@ -47,6 +48,7 @@ public:
Read,
Write,
Flush,
Open,
};
Q_ENUM(Type)

19
src/corelib/io/qrandomaccessasyncfile.cpp
View File

@ -18,12 +18,6 @@ QRandomAccessAsyncFile::~QRandomAccessAsyncFile()
close();
}
bool QRandomAccessAsyncFile::open(const QString &filePath, QIODeviceBase::OpenMode mode)
{
Q_D(QRandomAccessAsyncFile);
return d->open(filePath, mode);
}
void QRandomAccessAsyncFile::close()
{
Q_D(QRandomAccessAsyncFile);
@ -36,6 +30,19 @@ qint64 QRandomAccessAsyncFile::size() const
return d->size();
}
/*!
\internal
Attempts to open the file \a filePath with mode \a mode.
\include qrandomaccessasyncfile.cpp returns-qiooperation
*/
QIOOperation *QRandomAccessAsyncFile::open(const QString &filePath, QIODeviceBase::OpenMode mode)
{
Q_D(QRandomAccessAsyncFile);
return d->open(filePath, mode);
}
/*!
\internal

2
src/corelib/io/qrandomaccessasyncfile_p.h
View File

@ -32,10 +32,10 @@ public:
~QRandomAccessAsyncFile() override;
// sync APIs
bool open(const QString &filePath, QIODeviceBase::OpenMode mode);
void close();
qint64 size() const;
[[nodiscard]] QIOOperation *open(const QString &filePath, QIODeviceBase::OpenMode mode);
[[nodiscard]] QIOOperation *flush();
// owning APIs: we are responsible for storing the data

14
src/corelib/io/qrandomaccessasyncfile_p_p.h
View File

@ -48,10 +48,10 @@ public:
void init();
void cancelAndWait(QIOOperation *op);
bool open(const QString &path, QIODeviceBase::OpenMode mode);
void close();
qint64 size() const;
[[nodiscard]] QIOOperation *open(const QString &path, QIODeviceBase::OpenMode mode);
[[nodiscard]] QIOOperation *flush();
[[nodiscard]] QIOReadOperation *read(qint64 offset, qint64 maxSize);
@ -78,6 +78,13 @@ public:
};
private:
enum class FileState : quint8
{
Closed,
OpenPending, // already got an open request
Opened,
};
mutable QBasicMutex m_engineMutex;
std::unique_ptr<QFSFileEngine> m_engine;
QFutureWatcher<OperationResult> m_watcher;
@ -86,9 +93,14 @@ private:
QPointer<QIOOperation> m_currentOperation;
qsizetype numProcessedBuffers = 0;
QString m_filePath;
QIODeviceBase::OpenMode m_openMode;
FileState m_fileState = FileState::Closed;
void executeNextOperation();
void processBufferAt(qsizetype idx);
void processFlush();
void processOpen();
void operationComplete();
#endif
};

86
src/corelib/io/qrandomaccessasyncfile_threadpool.cpp
View File

@ -96,17 +96,27 @@ void QRandomAccessAsyncFilePrivate::cancelAndWait(QIOOperation *op)
}
}
bool QRandomAccessAsyncFilePrivate::open(const QString &path, QIODeviceBase::OpenMode mode)
QIOOperation *
QRandomAccessAsyncFilePrivate::open(const QString &path, QIODeviceBase::OpenMode mode)
{
QMutexLocker locker(&m_engineMutex);
if (m_engine) {
// already opened!
return false;
// We generate the command in any case. But if the file is already opened,
// it will finish with an error
if (m_fileState == FileState::Closed) {
m_filePath = path;
m_openMode = mode;
m_fileState = FileState::OpenPending;
}
m_engine = std::make_unique<QFSFileEngine>(path);
mode |= QIODeviceBase::Unbuffered;
return m_engine->open(mode, std::nullopt);
auto *dataStorage = new QtPrivate::QIOOperationDataStorage();
auto *priv = new QIOOperationPrivate(dataStorage);
priv->type = QIOOperation::Type::Open;
auto *op = new QIOOperation(*priv, q_ptr);
m_operations.append(op);
executeNextOperation();
return op;
}
void QRandomAccessAsyncFilePrivate::close()
@ -127,11 +137,15 @@ void QRandomAccessAsyncFilePrivate::close()
cancelAndWait(m_currentOperation.get());
}
QMutexLocker locker(&m_engineMutex);
if (m_engine) {
m_engine->close();
m_engine.reset();
{
QMutexLocker locker(&m_engineMutex);
if (m_engine) {
m_engine->close();
m_engine.reset();
}
}
m_fileState = FileState::Closed;
}
qint64 QRandomAccessAsyncFilePrivate::size() const
@ -323,6 +337,9 @@ void QRandomAccessAsyncFilePrivate::executeNextOperation()
case QIOOperation::Type::Flush:
processFlush();
break;
case QIOOperation::Type::Open:
processOpen();
break;
case QIOOperation::Type::Unknown:
Q_ASSERT_X(false, "executeNextOperation", "Operation of type Unknown!");
// For release builds - directly complete the operation
@ -425,6 +442,38 @@ void QRandomAccessAsyncFilePrivate::processFlush()
m_watcher.setFuture(f);
}
void QRandomAccessAsyncFilePrivate::processOpen()
{
Q_ASSERT(!m_currentOperation.isNull());
auto *priv = QIOOperationPrivate::get(m_currentOperation.get());
auto &dataStorage = priv->dataStorage;
Q_ASSERT(dataStorage->isEmpty());
QFuture<OperationResult> f;
if (m_fileState == FileState::OpenPending) {
// create the engine
m_engineMutex.lock();
m_engine = std::make_unique<QFSFileEngine>(m_filePath);
m_engineMutex.unlock();
QBasicMutex *mutexPtr = &m_engineMutex;
auto op = [engine = m_engine.get(), mutexPtr, mode = m_openMode] {
QRandomAccessAsyncFilePrivate::OperationResult result{0, QIOOperation::Error::None};
QMutexLocker locker(mutexPtr);
const bool res =
engine && engine->open(mode | QIODeviceBase::Unbuffered, std::nullopt);
if (!res)
result.error = QIOOperation::Error::Open;
return result;
};
f = QtFuture::makeReadyVoidFuture().then(asyncFileThreadPool(), op);
} else {
f = QtFuture::makeReadyVoidFuture().then(asyncFileThreadPool(), [] {
return QRandomAccessAsyncFilePrivate::OperationResult{0, QIOOperation::Error::Open};
});
}
m_watcher.setFuture(f);
}
void QRandomAccessAsyncFilePrivate::operationComplete()
{
// TODO: if one of the buffers was read/written with an error,
@ -484,6 +533,19 @@ void QRandomAccessAsyncFilePrivate::operationComplete()
case QIOOperation::Type::Flush:
priv->operationComplete(res.error);
break;
case QIOOperation::Type::Open:
if (m_fileState == FileState::OpenPending) {
if (res.error == QIOOperation::Error::None) {
m_fileState = FileState::Opened;
} else {
m_fileState = FileState::Closed;
m_engineMutex.lock();
m_engine.reset();
m_engineMutex.unlock();
}
}
priv->operationComplete(res.error);
break;
case QIOOperation::Type::Unknown:
priv->setError(QIOOperation::Error::Aborted);
break;

112
tests/auto/corelib/io/qrandomaccessasyncfile/tst_qrandomaccessasyncfile.cpp
View File

@ -9,6 +9,8 @@
#include <QtTest/qsignalspy.h>
#include <QtTest/qtest.h>
using namespace Qt::StringLiterals;
template <typename T>
static bool spanIsEqualToByteArray(QSpan<T> lhs, QByteArrayView rhs) noexcept
{
@ -43,6 +45,9 @@ private Q_SLOTS:
void readWriteNoBuffers_data();
void readWriteNoBuffers();
void asyncOpenErrors();
void closeCornerCases();
private:
enum class ReadWriteOp : quint8
{
@ -93,7 +98,9 @@ void tst_QRandomAccessAsyncFile::size()
// File not opened -> size unknown
QCOMPARE_EQ(file.size(), -1);
QVERIFY(file.open(m_file.fileName(), QIODeviceBase::ReadOnly));
QIOOperation *openOp = file.open(m_file.fileName(), QIODeviceBase::ReadOnly);
QTRY_COMPARE(openOp->isFinished(), true);
QCOMPARE_EQ(openOp->error(), QIOOperation::Error::None);
QCOMPARE(file.size(), FileSize);
}
@ -101,7 +108,9 @@ void tst_QRandomAccessAsyncFile::size()
void tst_QRandomAccessAsyncFile::roundtripOwning()
{
QRandomAccessAsyncFile file;
QVERIFY(file.open(m_file.fileName(), QIODevice::ReadWrite));
QIOOperation *openOp = file.open(m_file.fileName(), QIODeviceBase::ReadWrite);
QTRY_COMPARE(openOp->isFinished(), true);
QCOMPARE_EQ(openOp->error(), QIOOperation::Error::None);
// All operations will be deleted together with the file
@ -166,7 +175,9 @@ void tst_QRandomAccessAsyncFile::roundtripOwning()
void tst_QRandomAccessAsyncFile::roundtripNonOwning()
{
QRandomAccessAsyncFile file;
QVERIFY(file.open(m_file.fileName(), QIODevice::ReadWrite));
QIOOperation *openOp = file.open(m_file.fileName(), QIODeviceBase::ReadWrite);
QTRY_COMPARE(openOp->isFinished(), true);
QCOMPARE_EQ(openOp->error(), QIOOperation::Error::None);
// All operations will be deleted together with the file
@ -240,7 +251,9 @@ void tst_QRandomAccessAsyncFile::roundtripNonOwning()
void tst_QRandomAccessAsyncFile::roundtripVectored()
{
QRandomAccessAsyncFile file;
QVERIFY(file.open(m_file.fileName(), QIODevice::ReadWrite));
QIOOperation *openOp = file.open(m_file.fileName(), QIODeviceBase::ReadWrite);
QTRY_COMPARE(openOp->isFinished(), true);
QCOMPARE_EQ(openOp->error(), QIOOperation::Error::None);
// All operations will be deleted together with the file
@ -295,7 +308,9 @@ void tst_QRandomAccessAsyncFile::roundtripVectored()
void tst_QRandomAccessAsyncFile::readLessThanMax()
{
QRandomAccessAsyncFile file;
QVERIFY(file.open(m_file.fileName(), QIODeviceBase::ReadOnly));
QIOOperation *openOp = file.open(m_file.fileName(), QIODeviceBase::ReadOnly);
QTRY_COMPARE(openOp->isFinished(), true);
QCOMPARE_EQ(openOp->error(), QIOOperation::Error::None);
constexpr qint64 offsetFromEnd = 100;
@ -364,7 +379,9 @@ void tst_QRandomAccessAsyncFile::readLessThanMax()
void tst_QRandomAccessAsyncFile::flushIsBarrier()
{
QRandomAccessAsyncFile file;
QVERIFY(file.open(m_file.fileName(), QIODevice::ReadWrite));
QIOOperation *openOp = file.open(m_file.fileName(), QIODeviceBase::ReadWrite);
QTRY_COMPARE(openOp->isFinished(), true);
QCOMPARE_EQ(openOp->error(), QIOOperation::Error::None);
// All operations will be deleted together with the file
@ -483,8 +500,11 @@ void tst_QRandomAccessAsyncFile::errorHandling()
QFETCH(const QIOOperation::Error, expectedError);
QRandomAccessAsyncFile file;
if (expectedError != QIOOperation::Error::FileNotOpen)
QVERIFY(file.open(m_file.fileName(), openMode));
if (expectedError != QIOOperation::Error::FileNotOpen) {
QIOOperation *openOp = file.open(m_file.fileName(), openMode);
QTRY_COMPARE(openOp->isFinished(), true);
QCOMPARE_EQ(openOp->error(), QIOOperation::Error::None);
}
QIOOperation *op = nullptr;
if (operation == QIOOperation::Type::Read)
@ -518,7 +538,11 @@ void tst_QRandomAccessAsyncFile::fileClosedInProgress()
QFETCH(const QIOOperation::Type, operation);
QRandomAccessAsyncFile file;
QVERIFY(file.open(m_file.fileName(), QIODevice::ReadWrite));
if (operation != QIOOperation::Type::Open) {
QIOOperation *openOp = file.open(m_file.fileName(), QIODeviceBase::ReadWrite);
QTRY_COMPARE(openOp->isFinished(), true);
QCOMPARE_EQ(openOp->error(), QIOOperation::Error::None);
}
constexpr qint64 OneMb = 1024 * 1024;
std::array<QIOOperation *, 5> operations;
@ -543,6 +567,8 @@ void tst_QRandomAccessAsyncFile::fileClosedInProgress()
}
} else if (operation == QIOOperation::Type::Flush) {
op = file.flush();
} else if (operation == QIOOperation::Type::Open) {
op = file.open(m_file.fileName(), QIODeviceBase::ReadWrite);
}
QVERIFY(op);
operations[i] = op;
@ -575,7 +601,11 @@ void tst_QRandomAccessAsyncFile::fileRemovedInProgress()
{
QRandomAccessAsyncFile file;
QVERIFY(file.open(m_file.fileName(), QIODevice::ReadWrite));
if (operation != QIOOperation::Type::Open) {
QIOOperation *openOp = file.open(m_file.fileName(), QIODeviceBase::ReadWrite);
QTRY_COMPARE(openOp->isFinished(), true);
QCOMPARE_EQ(openOp->error(), QIOOperation::Error::None);
}
for (size_t i = 0; i < operations.size(); ++i) {
const qint64 offset = i * OneMb;
@ -596,6 +626,8 @@ void tst_QRandomAccessAsyncFile::fileRemovedInProgress()
}
} else if (operation == QIOOperation::Type::Flush) {
op = file.flush();
} else if (operation == QIOOperation::Type::Open) {
op = file.open(m_file.fileName(), QIODeviceBase::ReadWrite);
}
QVERIFY(op);
operations[i] = op;
@ -616,7 +648,11 @@ void tst_QRandomAccessAsyncFile::operationsDeletedInProgress()
QFETCH(const QIOOperation::Type, operation);
QRandomAccessAsyncFile file;
QVERIFY(file.open(m_file.fileName(), QIODevice::ReadWrite));
if (operation != QIOOperation::Type::Open) {
QIOOperation *openOp = file.open(m_file.fileName(), QIODeviceBase::ReadWrite);
QTRY_COMPARE(openOp->isFinished(), true);
QCOMPARE_EQ(openOp->error(), QIOOperation::Error::None);
}
constexpr qint64 OneMb = 1024 * 1024;
std::array<QIOOperation *, 5> operations;
@ -641,6 +677,8 @@ void tst_QRandomAccessAsyncFile::operationsDeletedInProgress()
}
} else if (operation == QIOOperation::Type::Flush) {
op = file.flush();
} else if (operation == QIOOperation::Type::Open) {
op = file.open(m_file.fileName(), QIODeviceBase::ReadWrite);
}
QVERIFY(op);
operations[i] = op;
@ -672,6 +710,7 @@ void tst_QRandomAccessAsyncFile::generateOperationColumns()
QTest::addRow("write_%s", v.name) << v.own << QIOOperation::Type::Write;
}
QTest::newRow("flush") << Ownership::NonOwning /* ignored */ << QIOOperation::Type::Flush;
QTest::newRow("open") << Ownership::NonOwning /* ignored */ << QIOOperation::Type::Open;
}
void tst_QRandomAccessAsyncFile::readWriteNoBuffers_data()
@ -694,7 +733,9 @@ void tst_QRandomAccessAsyncFile::readWriteNoBuffers()
QFETCH(const qint64, maxSize); // for OwningRead only
QRandomAccessAsyncFile file;
QVERIFY(file.open(m_file.fileName(), QIODeviceBase::ReadWrite));
QIOOperation *openOp = file.open(m_file.fileName(), QIODeviceBase::ReadWrite);
QTRY_COMPARE(openOp->isFinished(), true);
QCOMPARE_EQ(openOp->error(), QIOOperation::Error::None);
constexpr qint64 offset = 1024 * 1024;
QByteArray emptyBuffer;
@ -749,6 +790,53 @@ void tst_QRandomAccessAsyncFile::readWriteNoBuffers()
}
}
void tst_QRandomAccessAsyncFile::asyncOpenErrors()
{
QRandomAccessAsyncFile file;
// open with incorrect filename
{
auto *op = file.open(QString(), QIODeviceBase::ReadWrite);
QTRY_COMPARE_EQ(op->isFinished(), true);
QCOMPARE_EQ(op->error(), QIOOperation::Error::Open);
}
// second open() fails
{
auto *op1 = file.open(m_file.fileName(), QIODeviceBase::ReadOnly);
auto *op2 = file.open(m_file.fileName(), QIODeviceBase::WriteOnly);
QTRY_COMPARE_EQ(op1->isFinished(), true);
QCOMPARE_EQ(op1->error(), QIOOperation::Error::None);
QTRY_COMPARE_EQ(op2->isFinished(), true);
QCOMPARE_EQ(op2->error(), QIOOperation::Error::Open);
}
}
void tst_QRandomAccessAsyncFile::closeCornerCases()
{
// close() without open does not hang
{
QRandomAccessAsyncFile file;
file.close();
}
// re-open after close() works
{
QRandomAccessAsyncFile file;
auto *op = file.open(m_file.fileName(), QIODeviceBase::ReadWrite);
QTRY_COMPARE_EQ(op->isFinished(), true);
QCOMPARE_EQ(op->error(), QIOOperation::Error::None);
file.close();
op = file.open(m_file.fileName(), QIODeviceBase::ReadWrite);
QTRY_COMPARE_EQ(op->isFinished(), true);
QCOMPARE_EQ(op->error(), QIOOperation::Error::None);
}
}
QTEST_MAIN(tst_QRandomAccessAsyncFile)
#include "tst_qrandomaccessasyncfile.moc"