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: accessing wild pointer in qt6 #296

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

18202781743
Copy link
Contributor

qobject_cast<QWindow*>(window) is returned nullptr if window is destroyed in qt6, and we can see it on tst_QWindow::qobject_castOnDestruction() in tst_qwindow.cpp.
Object's address may be the same as before in qt6, memory may be optimized.

pms: BUG-299239

qobject_cast<QWindow*>(window) is returned nullptr if window is destroyed in qt6, and we can see it  on tst_QWindow::qobject_castOnDestruction() in tst_qwindow.cpp.
Object's address may be the same as before  in qt6,  memory may be optimized.

pms: BUG-299239
@18202781743 18202781743 requested a review from mhduiy January 8, 2025 13:34
@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

代码审查意见:

  1. MoveWindowHelper类的析构函数中,将qobject_cast替换为static_cast可能是因为parent()返回的类型已经确定,不需要进行类型转换。但是,如果parent()返回的类型不是QWindow*,那么使用static_cast可能会导致运行时错误。建议确认parent()返回的类型,或者使用dynamic_cast进行更安全的类型转换。

  2. DTreeLandPlatformWindowHelper类的析构函数中,添加了注释// see tst_qwindow.cpp tst_QWindow::qobject_castOnDestruction(),这表明存在一个相关的测试用例。建议查看该测试用例,了解为什么需要使用static_cast而不是qobject_cast

  3. DTreeLandPlatformWindowHelper类的析构函数中,windowMap.remove(static_cast<QWindow*>(parent()));这行代码使用了static_cast,但是没有检查parent()是否为nullptr。如果parent()nullptr,那么static_cast可能会导致未定义行为。建议在调用static_cast之前添加对parent()是否为nullptr的检查。

  4. 代码中没有明显的语法或逻辑错误。

  5. 代码中没有明显的性能问题。

  6. 代码中没有明显的安全问题。

deepin-ci-robot added a commit to linuxdeepin/dtk6gui that referenced this pull request Jan 8, 2025
Synchronize source files from linuxdeepin/dtkgui.

Source-pull-request: linuxdeepin/dtkgui#296
Copy link
Contributor

@mhduiy mhduiy left a comment

Choose a reason for hiding this comment

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

QObject::destroyed后代表该对象即将被真正销毁,对象的状态不稳定,操作对象可能会导致一些无法预测的错误(qobject_cast类型安全检查无法通过,即转换为nullptr)且这里不需要再使用该对象,仅移除对象在QList中存储的的指针,所以使用static_cast进行强制类型转换

我这样理解对吗,如果ok,改动合理

@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, mhduiy

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mhduiy
Copy link
Contributor

mhduiy commented Jan 8, 2025

不过为什么强调仅在Qt6下会出现这样的情况

@18202781743
Copy link
Contributor Author

18202781743 commented Jan 9, 2025

不过为什么强调仅在Qt6下会出现这样的情况

qt6下,1.明确了qwindow在析构后,qobject_cast返回的的是null,
image

2.qt6应该是做了内存优化,短暂时间内重复new出的qt对象,其address可能相同,

我们这里出现的情况是,传入了之前析构了的window相同的address,但windowMap没有移除已经析构了的helper,导致出现了访问wild pointer的问题,

@18202781743 18202781743 merged commit 7362e04 into linuxdeepin:master Jan 9, 2025
22 of 24 checks passed
18202781743 pushed a commit to linuxdeepin/dtk6gui that referenced this pull request Jan 9, 2025
Synchronize source files from linuxdeepin/dtkgui.

Source-pull-request: linuxdeepin/dtkgui#296
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.

3 participants