Skip to content
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.

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

Fix crash due to calling QWidget::move from a non-GUI thread while ex… #5293

Merged
merged 1 commit into from
Oct 31, 2019

Conversation

Cyp
Copy link
Contributor

@Cyp Cyp commented Oct 31, 2019

…porting tracks.

Using signals/slots instead should be thread safe.

Crash callstack:

QWidget::move
SongEditor::updatePosition
Song::stop
Song::stopExport
ProjectRenderer::run
QThreadPrivate::start

…porting tracks.

Calling via QMetaObject::invokeMethod should be thread safe.

Crash callstack:

QWidget::move
SongEditor::updatePosition
Song::stop
Song::stopExport
ProjectRenderer::run
QThreadPrivate::start
@Cyp
Copy link
Contributor Author

Cyp commented Oct 31, 2019

Bah, apparently signals are protected in old Qt 4…

@Cyp Cyp force-pushed the fix-sporadic-export-crash branch from 2e7a9d3 to 2e90398 Compare October 31, 2019 09:03
@lukas-w
Copy link
Member

lukas-w commented Oct 31, 2019

@Cyp Thanks for your PR. Can you provide steps to trigger the crash so we can test this?

@Cyp
Copy link
Contributor Author

Cyp commented Oct 31, 2019

It's not very reproducible — think it's only crashed 2 times so far for me — got the stack trace the second time.

The steps are to open something with lots of tracks, press Ctrl+Shift+E and export the tracks. Maybe try to mess around with the GUI while it's exporting, not sure whether that's needed. Then repeat maybe a few hundred times until it crashes.

@lukas-w
Copy link
Member

lukas-w commented Oct 31, 2019

@Cyp Understood. I can't reproduce a crash, but the change makes sense and should be safe. Merging 👍

@lukas-w lukas-w merged commit fd203c3 into LMMS:stable-1.2 Oct 31, 2019
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants