stevan / promises-perl

An implementation of Promises in Perl
31 stars 29 forks source link

Protect from circular references with inside-out storage for callbacks #32

Closed kraih closed 9 years ago

kraih commented 10 years ago

Here's a pull request, as requested on IRC. Storing the callbacks outside the promise object protects from many commonly occuring circular references with closures. Sorry for the lack of unit tests, can't think of a good test case at the moment.

stevan commented 10 years ago

@clintongormley want to take a quick look at this?

clintongormley commented 10 years ago

This looks good to me. Note: the use of next_tick requires a bump in Mojo version. Also, I'm using this module in 5.8 (without the Mojo part), so would prefer not to use the // operator.

Also wondering if we should store result in a fieldhash as well, for the same reasons?

paulgrove commented 10 years ago

I've been using kraih's branch for a while now without issues, and would +1 to see this pull request merged.

stevan commented 10 years ago

@og01 honestly, at this moment it is about if I have time or not (which I currently do not), so if you would be willing to help apply the changes @clintongormley mentioned and do a little release engineering (updating changelog, etc), I would be eternally grateful!

kraih commented 9 years ago

Closed this pull request since i no longer recommend handling callbacks this way. The circular references prevented by it are mostly harmless and will not lead to memory leaks. You could argue that hiding a few false positives from Devel::Cycle is worth it, but i'm not sure about that yet.

stevan commented 9 years ago

@kraih how do you recommend handing callbacks these days?

kraih commented 9 years ago

@stevan Just like you do now, explicitly clear $deferred->{resolved} and $deferred->{rejected} once the promise has been resolved to break all cycles.

stevan commented 9 years ago

Oh, I thought you meant not handling callbacks with Promises at all, but still that is a good bit of advice.