ahocevar at opengeo.org
Fri Jan 9 07:51:19 EST 2009
Erik Uzureau wrote:
> This is an interesting proposition.
> I have wanted for a long time to reform the way we do destroy().
> I've wanted to go through and make a patch replacing all the
> this.propX = null;
> delete this.propX;
> ...but have sort of just never gotten around to it.
> as for the proposition here, two thoughts:
> 1) overriding the valueOf() thing makes me a little nervous. It seems
> like this is the main reason for you suggesting this in the first
> place, and I can see how this would be handy... but my gut reaction is
> that having to special case our code to deal with a variable that has
> possibly been destroy()ed... it sounds like that's not a situation
> we'd ever want to be in. the response to that might be "keep on
> dreamin' uz, have you looked at the code lately?" and i'm happy to
> stomach that ;-).... but seriously, in my mind, the destroy() call in
> my mind should only be happenning when the variable in question is
> absolutely no longer needed. which to me means that there should never
> be code calling it at that point. maybe there are some cases i'm not
> thinking of (event callbacks?) but anyways, just a gut feeling. as
> always, more than happy to be convinced otherwise.
You're absolutely right Erik. The last time we revisited issues with
destroy() was before OpenLayers 2.6, when we tried to settle cases where
we were unsure if an object had already been destroyed, and succeeded in
some cases. I still think that this should be our goal.
But in real life this goal might be very hard to achieve, and I cannot
neglect the elegance of this valueOf() override. But I am asking myself
what happens to properties that delete fails on (e.g. because they are
still referenced somewhere)? I fear memory leaks here, which is another
strong argument to aim at resolving destroy race conditions instead of
working around them.
More information about the Dev