-
Notifications
You must be signed in to change notification settings - Fork 113
bugfix: reslove release error of MemoryMapping. #686
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
bugfix: reslove release error of MemoryMapping. #686
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
本次变更修复了一个严重的内存管理错误,即在 destroy_memory_mapping 函数中错误地使用了 free() 来释放由 std::make_unique (内部使用 new) 分配的内存。将 free(mapping) 修改为 delete mapping 是完全正确的,它解决了 C 和 C++ 内存管理方式混用导致的服务崩溃问题。此修复直接且有效,没有发现其他问题。
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
此拉取请求正确地识别并解决了一个位于 state_dict.cpp 中的严重内存管理错误。使用 new(通过 std::make_unique)分配内存,却使用 free 来释放,这是一个典型的导致未定义行为的 C++ 陷阱。将 free 修改为 delete 是正确的修复方法。此更改有效地解决了报告中提到的段错误问题。
|
您好,为了风格一致性,请尽量把commit message改为英文的(bugfix: xxx),包括PR标题。 |
RobbieLeung
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Issue: destroy_memory_mapping() uses free() to release memory allocated by new (std::make_unique), causing segmentation fault when loading models. Fix: Change free(mapping) to delete mapping to use correct memory deallocation method. Impact: Fixes server crash during model loading at startup.
5aebda5 to
8ed84a0
Compare
Issue
Server crashes with segmentation fault when loading models at startup.
Error stack:
Root Cause
destroy_memory_mapping()usesfree()to release memory allocated bystd::make_unique(i.e.,new), mixing C++ and C memory management, causing jemalloc to trigger segmentation fault.Fix
Change
free(mapping)todelete mapping.Impact