Opened 7 years ago

Closed 7 years ago

#11647 closed Feature Requests (worksforme)

add cleanup expired connections API

Reported by: Lutts Cao <lutts.cao@…> Owned by: Frank Mori Hess
Milestone: To Be Determined Component: signals2
Version: Boost 1.59.0 Severity: Problem
Keywords: Cc:

Description

when use signals2 with tracked shared object, it will cause memory leak when the shared track object which is allocated with allocated_shared() or make_shared() is destructed

another problem is: after the shared track object is destructed, signals2::connection::connected() will return false, and signals2::connection::disconnect() will do nothing

according to signals2's code, expired connection cleanup will happen after operator() is called, and maybe happen when signal.connect() is called. there is a problem when the signal is rarely emited, which means the time between successive operator() calls is long

I have tried boost 1.59.0, expired connection cause memory leak is still a problem.

I suggested to add an explicit cleanup API to signals2::signal class to let the user cleanup expired connections explicitly

or if you think add such an API is not a must, may be you can add a comment in boost.signals2 document to warn the users that there may be memory leak.

make_shared() and allocated_shared() in boost

document can be found at http://www.boost.org/doc/libs/1_59_0/libs/smart_ptr/make_shared.html

it is explicitly said these two function templates will use a single allocation, eliminating a significant portion of shared_ptr's construction overhead. This eliminates one of the major efficiency complaints about shared_ptr

make_shared() and allocated_shared() in C++11 standard spec =

in C++11 standard section 20.7.2.2.6.6 at page 573: Implementations are encouraged, but not required, to perform no more than one memory allocation.

make_shared() and allocated_shared() in Compiler implementations

as far as I know, GCC, CLang and Microsoft Visual Studio all use single allocation

Change History (3)

comment:1 by Frank Mori Hess, 7 years ago

Are you sure you are using 1.59? Because when I run the test program from your previous ticket against git develop branch (1.59 should behave similarly), I get the following, which shows the memory getting deallocated the first time you call connected() after the tracked object is destroyed.

---TestCase2: check signal2 will prevent memory from being freed---
allocate 1 objects, object size is 72
construct TrackObject
---TestCase2: after registerHandler: conn.connected: 1
signal triggered
destruct TrackObject
---TestCase2: should free trackobj here, but not---
deallocate 1 objects
---TestCase2: connected: 0
---TestCase2: connected() shows that it's already disconnected
--TestCase2: so, conn.disconnect() does nothing, the memory is still not freed
---TestCase2: only re-trigger will free memory before this line ---

in reply to:  1 comment:2 by Lutts Cao <lutts.cao@…>, 7 years ago

Sorry, I just checked I STILL use 1.56 which is installed in my system, not 1.59.0 which is build my self

Seems you must call some method on signals2::connection or signals2::signal to cleanup the expired connections, for example, call connection.connected() or signal.num_slots()

But this implicit behavior is not documented, I suggest add some comment in the document

Thanks for your work!

Replying to fmhess:

Are you sure you are using 1.59? Because when I run the test program from your previous ticket against git develop branch (1.59 should behave similarly), I get the following, which shows the memory getting deallocated the first time you call connected() after the tracked object is destroyed.

---TestCase2: check signal2 will prevent memory from being freed---
allocate 1 objects, object size is 72
construct TrackObject
---TestCase2: after registerHandler: conn.connected: 1
signal triggered
destruct TrackObject
---TestCase2: should free trackobj here, but not---
deallocate 1 objects
---TestCase2: connected: 0
---TestCase2: connected() shows that it's already disconnected
--TestCase2: so, conn.disconnect() does nothing, the memory is still not freed
---TestCase2: only re-trigger will free memory before this line ---

comment:3 by Frank Mori Hess, 7 years ago

Resolution: worksforme
Status: newclosed
Note: See TracTickets for help on using tickets.