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 sharedptr , weakptr implentation leak #1

Merged
merged 1 commit into from
Jun 13, 2024
Merged

Conversation

gulafaran
Copy link
Contributor

if we dont decrement the sharedptr on destruction any weakptr remaining will have an impl_ with a ref of 1 and upon destruction of the weakptr it wont delete the implentation because it thinks a shared pointer still exist.

as in

if (impl_ && impl_->ref() == 1)
destruct and have impl->ref () == 1;

if (impl_->wref() == 0 && impl_->ref() == 0 && !impl_->destroying()) {
weakptr will not delete it because it still has a ref above 0. but the sharedptr is actually dead.

destroyImpl() also already checks for weakptr ref before deleting impl so the comment section in the old destruction logic shouldnt be true.

leak can be seen in the hyprutils_memory test or hyprland itself

┤tom-legion->tom main ~/dev/hyprutils/build/Desktop-Debug
└➤ valgrind ./hyprutils_memory --leak-check=full
==43588== Memcheck, a memory error detector
==43588== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==43588== Using Valgrind-3.23.1.GIT and LibVEX; rerun with -h for copyright info
==43588== Command: ./hyprutils_memory --leak-check=full
==43588== 
Passed *intPtr. Got 10
Passed intPtr.strongRef(). Got 1
Passed *intPtr. Got 10
Passed intPtr.strongRef(). Got 1
Passed *weak.get(). Got 10
Passed weak.expired(). Got 0
Passed weak.expired(). Got 1
==43588== 
==43588== HEAP SUMMARY:
==43588==     in use at exit: 32 bytes in 1 blocks
==43588==   total heap usage: 4 allocs, 3 frees, 74,788 bytes allocated
==43588== 
==43588== LEAK SUMMARY:
==43588==    definitely lost: 32 bytes in 1 blocks
==43588==    indirectly lost: 0 bytes in 0 blocks
==43588==      possibly lost: 0 bytes in 0 blocks
==43588==    still reachable: 0 bytes in 0 blocks
==43588==         suppressed: 0 bytes in 0 blocks
==43588== Rerun with --leak-check=full to see details of leaked memory
==43588== 
==43588== For lists of detected and suppressed errors, rerun with: -s
==43588== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

if we dont decrement the sharedptr on destruction any weakptr remaining
will have an impl_ with a ref of 1 and upon destruction of the weakptr
it wont delete the implentation because it thinks a shared pointer still
exist.
@gulafaran
Copy link
Contributor Author

this could be one of the puzzles of why hyprwm/Hyprland#6459 and hyprwm/Hyprland#6381 since quite a lot signal handling in hyprland etc are using sharedptr/weakptr and eventually turns into leaks quite fast

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

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

oh my, thanks!

@vaxerski vaxerski merged commit 1f4743a into hyprwm:main Jun 13, 2024
4 checks passed
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.

2 participants