Skip to content

Conversation

@Upliner
Copy link
Contributor

@Upliner Upliner commented Oct 21, 2024

When system doesn't have enough VRAM, vkAllocateMemory fails and my app crashes. This patch fixes this by always checking return value of allocate_memory and allocate_dedicated_memory so my app can properly report error instead of crashing.

@github-actions github-actions bot added the core label Oct 21, 2024
@codecov-commenter
Copy link

codecov-commenter commented Nov 7, 2024

Codecov Report

❌ Patch coverage is 15.62500% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.28%. Comparing base (419db5e) to head (2212de3).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
src/allocator.cpp 15.62% 27 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5754      +/-   ##
==========================================
- Coverage   93.29%   93.28%   -0.01%     
==========================================
  Files         845      845              
  Lines      265560   265757     +197     
==========================================
+ Hits       247743   247905     +162     
- Misses      17817    17852      +35     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tencent-adm
Copy link
Member

tencent-adm commented Aug 29, 2025

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ Upliner
❌ nihui
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions
Copy link

github-actions bot commented Aug 29, 2025

The binary size change of libncnn.so (bytes)

architecture base size pr size difference
x86_64 15155464 15155464 0 😘
armhf 6182208 6182336 +128 ⚠️
aarch64 9520608 9454976 -65632 😘

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request fixes crashes that occur when Vulkan memory allocation fails due to insufficient VRAM by adding proper error handling and cleanup.

  • Adds null checks after all allocate_memory and allocate_dedicated_memory calls
  • Ensures proper resource cleanup (Vulkan object destruction and memory deallocation) on allocation failure
  • Prevents crashes by returning null instead of proceeding with invalid memory

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nihui nihui merged commit 1ce488e into Tencent:master Jan 9, 2026
144 of 152 checks passed
@nihui
Copy link
Member

nihui commented Jan 9, 2026

Thanks for your contribution !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants