Skip to content

fix(asyncimageresponse): fetch remote resources in the same thread as Account #8848

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
mgallien merged 1 commit into from
Oct 8, 2025
Merged

fix(asyncimageresponse): fetch remote resources in the same thread as Account #8848

mgallien merged 1 commit into from
Oct 8, 2025

Conversation

@nilsding
Copy link
Member

@nilsding nilsding commented Oct 8, 2025 •
edited
Loading

With this change some warnings from QObject like these are gone now:

QObject: Cannot create children for a parent that is in a different thread.

I suspect this might have been causing some "random" crashes during normal usage as well. QNAMs are supposed to be used from the same thread they were created in, which (as far as I could tell anyway) isn't the case within async image providers...

This change is similar to how the ImageResponse class inside src/gui/tray/usermodel.cpp fetches a remote resource:

desktop/src/gui/tray/usermodel.cpp

Lines 1772 to 1784 in fb20bb2

const auto account = accountState->account();
const auto qnam = account->networkAccessManager();
QMetaObject::invokeMethod(qnam, [this, requestedSize, avatarUserId, account]() {
const auto avatarSize = requestedSize.width() > 0 ? requestedSize.width() : 64;
const auto avatarJob = new AvatarJob(account, avatarUserId, avatarSize);
connect(avatarJob, &AvatarJob::avatarPixmap, this, [&](const QImage &avatarImg) {
QMetaObject::invokeMethod(this, [this, avatarImg] {
handleDone(AvatarJob::makeCircularAvatar(avatarImg));
});
});
avatarJob->start();
});

@nilsding
fix(asyncimageresponse): fetch remote resources in the same thread as…
89d5df6 
… Account

With this change some warnings from QObject like these are gone now:

    QObject: Cannot create children for a parent that is in a different thread.

I suspect this might have been causing some "random" crashes during
normal usage as well.  QNAMs are supposed to be used from the same
thread they were created in, which (as far as I could tell anyway) isn't
the case within async image providers...

This change is similar to how the `ImageResponse` class inside
`src/gui/tray/usermodel.cpp` fetches a remote resource.

Signed-off-by: Jyrki Gadinger <nilsding@nilsding.org>
@nilsding nilsding added this to the 4.0.0 milestone Oct 8, 2025
@nilsding nilsding self-assigned this Oct 8, 2025
@github-actions
Copy link

github-actions bot commented Oct 8, 2025

Artifact containing the AppImage: nextcloud-appimage-pr-8848.zip

Digest: sha256:03f92c1cdd01cb9c201dca26a80967593eeef859e17c06534311bbb08b942d0b

To test this change/fix you can download the above artifact file, unzip it, and run it.

Please make sure to quit your existing Nextcloud app and backup your data.

@nilsding
Copy link
Member Author

nilsding commented Oct 8, 2025

/backport to stable-4.0

Aiiaiiio
Aiiaiiio approved these changes Oct 8, 2025
View reviewed changes
Copy link
Collaborator

@Aiiaiiio Aiiaiiio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@mgallien mgallien merged commit 80d79e4 into master Oct 8, 2025
19 of 20 checks passed
@mgallien mgallien deleted the bugfix/noid/asyncimageresponse-use-qnam-thread branch October 8, 2025 15:43
@mgallien mgallien modified the milestones: 4.0.0, 4.1.0 Oct 8, 2025
@backportbot backportbot bot mentioned this pull request Oct 8, 2025
Merged
@nilsding
Copy link
Member Author

nilsding commented Oct 9, 2025

/backport to stable-3.17

@backportbot backportbot bot mentioned this pull request Oct 9, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Aiiaiiio Aiiaiiio Aiiaiiio approved these changes

@camilasan camilasan Awaiting requested review from camilasan camilasan is a code owner

@i2h3 i2h3 Awaiting requested review from i2h3 i2h3 is a code owner

@mgallien mgallien Awaiting requested review from mgallien mgallien is a code owner

Assignees

@nilsding nilsding

Labels

3. to review

Projects

None yet

Milestone

4.1.0

Development

Successfully merging this pull request may close these issues.

4 participants

@nilsding @Aiiaiiio @mgallien