search

ajvincent / es-membrane

An ECMAScript implementation of a Membrane, allowing users to dynamically hide, override, or extend objects in JavaScript with controlled effects on the original objects.
ISC License
109 stars 13 forks source link

Memory leak: ObjectGraphHandler.prototype.revokeEverything needs to drop references in ProxyMapping #166

Closed ajvincent closed 5 years ago

ajvincent commented 6 years ago

proxiedFields[graphName] needs to go away during any proxy revocation.

ajvincent commented 6 years ago

@erights Can you explain the memory leak to me again, please? The revokeEverything call revokes the proxies associated with a given ProxyMapping. Why is this insufficient?

ajvincent commented 5 years ago

Pushing to 0.10 for now: it doesn't seem to be an absolute blocker.

ajvincent commented 5 years ago

I should just end the debate by defining a singleton ProxyMapping.Dead = Symbol("dead proxy mapping"). Then revokeEverything can call Membrane.map.set(object, ProxyMapping.Dead), which will automatically break the last strong reference. For good measure, I can have Membrane.map.set(proxy, ProxyMapping.Dead) and Membrane.map.set(shadowTarget, ProxyMapping.Dead).

Deleting references in the WeakMap isn't good enough: that implies the possibility of allowing the Membrane to create new proxies.