fix: prevent crash when loadarchive#387
Conversation
Log: as title
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR fixes a crash when loading an archive by correcting the lambda capture in a deferred QTimer callback so that the archive path remains valid when loadArchive is invoked. Sequence diagram for deferred archive loading with QTimersequenceDiagram
participant MainWindow
participant QTimer
MainWindow->>QTimer: singleShot(200, lambda(path))
activate QTimer
QTimer-->>MainWindow: invoke lambda(path)
deactivate QTimer
MainWindow->>MainWindow: loadArchive(path)
Class diagram for MainWindow archive loading methodsclassDiagram
class MainWindow {
+bool handleArguments_Open(listParam: QStringList)
+void loadArchive(path: QString)
}
class QTimer {
+static void singleShot(msec: int, receiver: QObject, member: Pointer)
}
MainWindow ..> QTimer : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/source/mainwindow.cpp" line_range="2613" />
<code_context>
firstLoad = false;
auto path = listParam[0];
- QTimer::singleShot(200, [this, &path]() {
+ QTimer::singleShot(200, [this, path]() {
loadArchive(path);
});
</code_context>
<issue_to_address>
**issue (bug_risk):** Use a QObject context or a safe pointer instead of capturing `this` in a delayed lambda to avoid potential use-after-free.
Because this runs 200 ms later without a QObject context, `this` may already be destroyed (e.g., if the window closes quickly), leading to a use-after-free in `loadArchive(path)`. Please use the `QTimer::singleShot` overload that takes a QObject context, or capture a `QPointer<MainWindow>` and check it before calling `loadArchive`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| firstLoad = false; | ||
| auto path = listParam[0]; | ||
| QTimer::singleShot(200, [this, &path]() { | ||
| QTimer::singleShot(200, [this, path]() { |
There was a problem hiding this comment.
issue (bug_risk): Use a QObject context or a safe pointer instead of capturing this in a delayed lambda to avoid potential use-after-free.
Because this runs 200 ms later without a QObject context, this may already be destroyed (e.g., if the window closes quickly), leading to a use-after-free in loadArchive(path). Please use the QTimer::singleShot overload that takes a QObject context, or capture a QPointer<MainWindow> and check it before calling loadArchive.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: LiHua000, max-lvs The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Log: as title
Summary by Sourcery
Bug Fixes: