refactor: remove attach/detach thread management#1240
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我在这里给出了一些整体性的反馈：
- 既然已经移除了 Android 线程 attach/detach 的职责，建议在调用
AndroidNativeControlUnitMgr::screencap和dispatch_input_message的地方，通过注释或运行时断言把新的线程假设显式表达出来，这样可以清楚地表明它们必须在已附加线程中被调用。 - 由于
AttachThread/DetachThread不再从外部库中加载，可能值得检查一下是否还存在对这些符号的剩余引用（例如在原生库或构建配置中），以便清理掉它们，避免关于未使用入口点的困惑。
给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- 既然已经移除了 Android 线程 attach/detach 的职责，建议在调用 `AndroidNativeControlUnitMgr::screencap` 和 `dispatch_input_message` 的地方，通过注释或运行时断言把新的线程假设显式表达出来，这样可以清楚地表明它们必须在已附加线程中被调用。
- 由于 `AttachThread`/`DetachThread` 不再从外部库中加载，可能值得检查一下是否还存在对这些符号的剩余引用（例如在原生库或构建配置中），以便清理掉它们，避免关于未使用入口点的困惑。帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- Now that the Android thread attach/detach responsibilities are removed, consider making the new threading assumption explicit (e.g., via a comment or runtime assertion) at the call sites to
AndroidNativeControlUnitMgr::screencapanddispatch_input_messageso it’s clear they must be invoked from an already-attached thread. - Since
AttachThread/DetachThreadare no longer loaded from the external library, it may be worth checking whether any remaining references to those symbols (e.g., in native libraries or build configs) can be cleaned up to avoid confusion about unused entry points.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Now that the Android thread attach/detach responsibilities are removed, consider making the new threading assumption explicit (e.g., via a comment or runtime assertion) at the call sites to `AndroidNativeControlUnitMgr::screencap` and `dispatch_input_message` so it’s clear they must be invoked from an already-attached thread.
- Since `AttachThread`/`DetachThread` are no longer loaded from the external library, it may be worth checking whether any remaining references to those symbols (e.g., in native libraries or build configs) can be cleaned up to avoid confusion about unused entry points.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
|
干嘛用的，不要这玩意了？ |
Member
Author
对，我内化到Android实现上了，保证fw这边纯净一点 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
从控制单元 API 及其使用位置中移除 Android 特定的线程 attach/detach 处理逻辑。
Enhancements:
attach_thread/detach_threadAPI 以及相关的ScopedThreadAttach工具，简化 Android 原生控制单元接口。AttachThread/DetachThread函数指针和符号加载，清理动态 Android 外部库绑定。ControllerAgent和AndroidNativeControlUnitMgr的执行路径。Original summary in English
Summary by Sourcery
Remove Android-specific thread attach/detach handling from the control unit APIs and their usage sites.
Enhancements: