signals icon indicating copy to clipboard operation
signals copied to clipboard

Fix payload memory leak

Open mikezackles opened this issue 3 years ago • 7 comments

Previously, the sig_base destructor did not delete conn_base pointers if they were owned by a connection, but it did null their back references to the signal, which prevented them from destroying their payloads, which are placement-newed inside a vector owned by the sig_base instance. Thus payloads were leaked.

This commit introduces an explicit destroy_payload virtual function to conn_base so that the sig_base destructor can ensure that payloads are destroyed and still allow the owning connection to destroy the conn_base pointer.

This also fixes what seemed like a bug with conn_nontrivial destruction not handling blocked signals properly.

mikezackles avatar Mar 16 '21 01:03 mikezackles

@mikezackles do you happen to know why @TheWisp is inactive for about a year already? I'm worried a bit...

dumblob avatar May 31 '21 16:05 dumblob

No, I don't know why. If there is interest, I might be willing to consider maintaining a fork that includes my PRs, but I don't have a ton of time to commit.

mikezackles avatar May 31 '21 17:05 mikezackles

Ok. I'd say let's wait a few weeks and if @TheWisp won't show up, make a new issue in the issue tracker with the "development happens now in ... repository".

Do you have commit rights in this repository of @TheWisp ? If so, you could then lock all discussions to avoid mess during the "transition".

dumblob avatar May 31 '21 18:05 dumblob

Do you have commit rights in this repository of @TheWisp ?

No, I do not have commit rights.

Ok. I'd say let's wait a few weeks and if @TheWisp won't show up, make a new issue in the issue tracker with the "development happens now in ... repository".

Hmm, I'm pretty hesitant to make that type of announcement without hearing from the author. But perhaps at some point it would be OK to mention that there is an alternative fork if this project remains dormant.

My thought was just that I intend to continue using these changes anyway, so I'll be maintaining them one way or another (maybe just within my own projects).

But yes, let's give it some time for thought, and in the meantime it would be nice to hear from @TheWisp. Personally I'm appreciative of their work on this project and just wondering about what the best path forward is.

mikezackles avatar May 31 '21 19:05 mikezackles

Hi @dumblob and @mikezackles , thanks for caring about the project and my safety. A lot have happened last year - I moved country and change job to join one of the top software companies. I promise I'll take a look at it this weekend!!!

TheWisp avatar Jun 02 '21 09:06 TheWisp

Congratulations @TheWisp! And thanks for chiming in here!

Speaking of bus factor, have you considered giving few other devs enough permissions to this repository (or creating a separate GitHub organization for this purpose) to allow repo archiving, committing, and discussion locking?

dumblob avatar Jun 02 '21 10:06 dumblob

Yes, congratulations -- that's fantastic! Life is more important, so please don't feel rushed.

mikezackles avatar Jun 02 '21 14:06 mikezackles