QXlsx icon indicating copy to clipboard operation
QXlsx copied to clipboard

static thread_local QRegularExpression prevents closing QApplication after using Document in QtConcurrent::mapped

Open Dontsoft opened this issue 3 years ago • 1 comments

When using QXlsx::Document while running QtConcurrent::mapped like this:


constexpr double points_to_pixel(double points) { return points * 1.3333; }
constexpr double inch_to_pixel(double inch) { return inch * 96; }

struct CertificateTemplateDataLoader
{
    CertificateTemplateDataLoader(
        std::shared_ptr<DataStorage<CertificateTemplateData, QDateTime>> storage)
        : _storage(storage)
    {}

    using result_type = CertificateTemplateData;

    CertificateTemplateData operator()(const QString& file)
    {
        using namespace QXlsx;
        QFileInfo info(file);
        CertificateTemplateData templateData;
        if (!info.exists())
            return templateData;
        if (!_storage->needsUpdate(file, info.lastModified()))
            return _storage->get(file);
        QScopedPointer<Document> document(new Document(file));
        if (!document->load())
            return templateData;
        templateData.filename = file;
        templateData.sheetList = document->sheetNames();
        _storage->set(file, info.lastModified(), templateData);
        return templateData;
    }

    std::shared_ptr<DataStorage<CertificateTemplateData, QDateTime>> _storage;
};

QPointer<QFutureWatcher<CertificateTemplateData>> futureWatcher =
        new QFutureWatcher<CertificateTemplateData>(this);
    connect(futureWatcher.data(), &QFutureWatcher<CertificateTemplateData>::progressRangeChanged,
            this, &CertificateDialog::progressRangeChanged);
    connect(futureWatcher.data(), &QFutureWatcher<CertificateTemplateData>::progressValueChanged,
            ui->progressBar, &QProgressBar::setValue);
    connect(futureWatcher.data(), &QFutureWatcher<CertificateTemplateData>::finished, this, [=]() {
        auto loadedData = futureWatcher->future().results();
        for (const auto& dataEntry : loadedData)
        {
            if (dataEntry.filename.isEmpty())
                continue;
            _templateData.insert(dataEntry.filename, dataEntry);
            ui->templateComboBox->addItem(dataEntry.filename);
        }

        connect(ui->templateComboBox, qOverload<const QString&>(&QComboBox::currentIndexChanged),
                this, &CertificateDialog::templateIndexChanged);

        ui->stackedWidget->setCurrentIndex(0);
        if (loadedData.size() > 0)
        {
            ui->templateComboBox->setCurrentIndex(0);
            // templateIndexChanged(loadedData.first().filename);
        }
    });
    connect(futureWatcher.data(), &QFutureWatcher<CertificateTemplateData>::canceled, this,
            &CertificateDialog::canceled);

    connect(ui->cancelTemplateLoadingButton, &QPushButton::clicked, futureWatcher.data(),
            &QFutureWatcher<CertificateTemplateData>::cancel);

    connect(futureWatcher.data(), &QFutureWatcher<CertificateTemplateData>::finished,
            futureWatcher.data(), &QFutureWatcher<CertificateTemplateData>::deleteLater);
    connect(futureWatcher.data(), &QFutureWatcher<CertificateTemplateData>::canceled,
            futureWatcher.data(), &QFutureWatcher<CertificateTemplateData>::deleteLater);

    auto future = QtConcurrent::mapped(
        files, CertificateTemplateDataLoader(
                   infas::ServiceLocator<DataStorage<CertificateTemplateData, QDateTime>>::get()));
    futureWatcher->setFuture(future);

The following line prevents the application from being closed. Even though everything is closed my QApplication is somehow still running in the background (after using QXlsx::Document in QtConcurrent::mapped)

https://github.com/QtExcel/QXlsx/blob/b7b3d96fc78d69b9a24d9153e10abd395c77b143/QXlsx/source/xlsxcellreference.cpp#L106

Removing thread_local and using static only for the QRegularExpression object fixes the issue. Like this:

void CellReference::init(const QString& cell_str)
{
#if QT_VERSION >= QT_VERSION_CHECK(5, 0, 0)
    static QRegularExpression re(QStringLiteral("^\\$?([A-Z]{1,3})\\$?(\\d+)$"));
    QRegularExpressionMatch match = re.match(cell_str);
    if (match.hasMatch())
    {
        const QString col_str = match.captured(1);
        const QString row_str = match.captured(2);
        _row = row_str.toInt();
        _column = col_from_name(col_str);
    }
#else
    QRegExp re(QLatin1String("^\\$?([A-Z]{1,3})\\$?(\\d+)$"));
    if (re.indexIn(cell_str) != -1)
    {
        const QString col_str = re.cap(1);
        const QString row_str = re.cap(2);
        _row = row_str.toInt();
        _column = col_from_name(col_str);
    }
#endif
}

Dontsoft avatar Apr 07 '21 05:04 Dontsoft

Dear @Dontsoft

Thank you for reporting. If you can pull request, I will merge to qt6beta branch.

Sorry for the late reply. I have been working for a long time in a place with no internet.

j2doll avatar Apr 15 '21 12:04 j2doll