Removal of WeakMap/WeakSet clear

Andreas Rossberg rossberg at google.com
Thu Nov 27 06:44:25 PST 2014


On 27 November 2014 at 15:22, Andrea Giammarchi
<andrea.giammarchi at gmail.com> wrote:
> Andreas I think Mark few times already gave for granted WeakMaps are based
> on transposed representations and it's also partially pointless from a
> polyfill point of view to not implement them as such but clear makes fast
> polyfills very pointless with `.clear()` in place ( e.g. [1] )

I think you misunderstood. The argument in this thread that I was
responding to was that .clear is incompatible with a (presumably
desirable) transposed representation. That is not so, at least not for
any realistic implementation of that approach that I am aware of.

Polyfills are a different issue that I haven't considered. However,
the only functionally correct polyfill for WeakMaps is via non-weak
Maps or even arrays, and then there is no problem either.

> I've had similar thoughts when I've first realized there was a `delete`
> method since I thought: if you can delete, you should be able to iterate and
> get keys ... which wasn't even the initial case but that's how my initial
> polyfill was implemented, with a real `.clear()` ability that was making the
> WeakMap itself not weak anymore [2]

Well, there is no functionally correct polyfill for WeakMaps that is
actually weak, regardless of .clear.

> I start thinking for performance and polyfill sake *maybe* it's better for
> everyone to keep `.clear()` out of the equation so there won't be broken
> polyfill and implementations can decide which implementation is better for
> their main use cases.
>
> Who wants to expose clear can always do it via wrappers:
>
> ```js
> // just for example purpose
> function CleanableWeakMap() {
>   this.clean();
>   this.delete = this._wm.delete.bind(this._wm);
>   this.get = this._wm.get.bind(this._wm);
>   this.has = this._wm.has.bind(this._wm);
>   this.set = this._wm.set.bind(this._wm);
> }
> CleanableWeakMap.prototype.clean = function () {
>   this._wm = new WeakMap;
> };
> ```

Sure, but the same is true the other way round. So that argument
doesn't resolve the discussion either way.

/Andreas


> Above code should make everyone happy except the GC in between a fake
> `.clean()` and a garbage call but at least it could be sort of consistent
> across ES5 to ES.next platforms.
>
> Willing also to drop my non WeakMap and bring in either the Polymer polyfill
> which is already missing `.clear()` but polluting the global scope anyway
> with this "not really full spec" implementation, or the one in link [1]
>
>
> Regards
>
>
>
> [1] changing the slot without being able to clean up a thing:
>      https://gist.github.com/WebReflection/5991636#file-weakmap-js-L15-L19
>
> [2] not a WeakMap, you should delete things ( aka: sort of pointless WeakMap
> )
>
> https://github.com/WebReflection/es6-collections#the-weakmap-is-not-weak--and-why
>
>
>
> On Thu, Nov 27, 2014 at 9:17 AM, Andreas Rossberg <rossberg at google.com>
> wrote:
>>
>> The discussion here still makes various untested assumptions about the
>> transposed representation. With respect to .clear in particular, it
>> seems to assume that this representation does not normally need the
>> ability to iterate (internally) over the keys of a weak map. AFAICT,
>> that is incorrect. Both the implementation we discussed in the V8
>> team, and from what I heard, the implementation in IE, maintain a list
>> of all its keys for each weak map. Otherwise, when a map dies, you
>> couldn't generally kill the entries and the values associated with
>> them easily and in a timely manner. (And of course, this list means
>> almost twice the space usage for a map, because you duplicate part of
>> the information.)
>>
>> So from that perspective at least, .clear is not an issue.
>>
>> I also don't see the security issue, to be honest. In any
>> security-relevant (or abstraction-relevant) scenario you would be
>> crazy to hand out the (mutable!) weak map to third parties anyway.
>> They could run just as much havoc by _adding_ random things to the map
>> (or removing things at guess) as they could by clearing it.
>>
>> Mark, can you explain the specific scenario you have in mind?
>>
>> /Andreas
>>
>>
>> On 26 November 2014 at 20:21, Mark S. Miller <erights at google.com> wrote:
>> > On Wed, Nov 26, 2014 at 9:33 AM, Katelyn Gadd <kg at luminance.org> wrote:
>> >> Is there a detailed rationale for this somewhere?
>> >
>> > It is a combination of three issues.
>> >
>> > 1. The security issue.
>> > 2. The implementation issue.
>> > 3. The ES process issue.
>> >
>> > The implementation issue is that the use cases for WeakMaps basically
>> > divide into the following cases:
>> >
>> > a. Those for which we're confident that the map's lifetime outlives
>> > its typical key.
>> > b. Those for which we're confident that the key's lifetime outlives
>> > the typical map it is a key in.
>> > c. Those for which we're not confident which typically lives longer.
>> >
>> > For #a, the transposed representation of WeakMaps is strictly better.
>> > The non-transposed implementation would promote ephemeral keys to
>> > later GC generations, which is very expensive. (I believe the expense
>> > of this promotion has been radically underestimated in most prior
>> > discussions.) This is the GC pressure that really matters.
>> >
>> > For #b, just use a Map rather than a WeakMap.
>> >
>> > For #c, transposed rep or not is a tie. In the interests of minimizing
>> > implementation mechanism, we should just use a transposed rep for this
>> > as well.
>> >
>> > Given the transposed representation, the only implementation
>> > techniques for clear are
>> >
>> > x. Enumerating all memory
>> > y. Having the WeakMap encapsulate the token used to lookup the value
>> > in the key's hidden map, and have .clear replace this token.
>> >
>> > #x is not reasonable.
>> > #y is equivalent to the replacing of the Map that would be the
>> > user-level technique for emulating clear in any case. This is exactly
>> > what the use of WeakMaps for membranes do, when the membrane should be
>> > revoked. If .clear could be implemented for the transposed
>> > representation more efficiently than this, membranes would benefit
>> > from .clear as well. I have never expected they could.
>> >
>> >
>> > The process issue is that .clear was not in the original proposal (for
>> > all of these reasons), and it never achieved consensus in committee.
>> > It would have violated our process to keep it in the spec. The process
>> > issue is not "Should it be dropped?" since it was never legitimately
>> > added. The only issue is "Should it be added?".
>> >
>> >
>> >
>> >> Making typical
>> >> applications pay the cost here for a specific security scenario seems
>> >> really bizarre to me. Clearing standard library data structures is an
>> >> incredibly common operation. If you want to ensure that someone can't
>> >> clear the map/set, shouldn't you be handing them an encapsulated
>> >> version of the data structure? This seems like a corner case that
>> >> shouldn't justify removing an important primitive.
>> >>
>> >> If you have a clear method, the security problem seems solved by
>> >> wrapping it in an object or using a proxy to deny the ability to clear
>> >> (you hide the actual map/set, so it can't be cleared - you expose only
>> >> the operations you want to expose).
>> >>
>> >> If you don't have a clear method, anyone wanting to clear the data
>> >> structure has to throw it away and allocate a new one. This has
>> >> significant disadvantages:
>> >> The new structure starts empty at a default size, so repopulating it
>> >> will have to grow the buffer multiple times - this is undesirable for
>> >> cases where you are reusing a single data structure to store state for
>> >> a long-running application.
>> >> The allocation adds to GC and memory pressure for a long-running
>> >> application that needs to clear data structures frequently. Were it a
>> >> lightweight data type this would matter less, but a typical map
>> >> instance with data in it can occupy a considerable amount of space in
>> >> the heap.
>> >> Being able to clear the structure now requires that all consumers have
>> >> support for replacing their reference(s) to the old map with the new
>> >> one. This makes it harder to maintain encapsulation because you may
>> >> have saved a reference to the map in a private property or within a
>> >> closure. Now you need to add accessibility points to everything that
>> >> might retain the map so that you can update the reference. Or, you
>> >> have to encapsulate maps and sets just to recreate the clear operation
>> >> that should have been there to begin with.
>> >>
>> >> In either case, encapsulation or shielding the container behind a
>> >> proxy is necessary. I insist that the common case is the one that
>> >> shouldn't have to encapsulate, because optimizing for that case will
>> >> benefit the vast majority of web applications that use it and the
>> >> penalty to security-sensitive cases is small.
>> >>
>> >> -kg
>> >> _______________________________________________
>> >> es-discuss mailing list
>> >> es-discuss at mozilla.org
>> >> https://mail.mozilla.org/listinfo/es-discuss
>> >
>> >
>> >
>> > --
>> >     Cheers,
>> >     --MarkM
>> > _______________________________________________
>> > es-discuss mailing list
>> > es-discuss at mozilla.org
>> > https://mail.mozilla.org/listinfo/es-discuss
>> _______________________________________________
>> es-discuss mailing list
>> es-discuss at mozilla.org
>> https://mail.mozilla.org/listinfo/es-discuss
>
>


More information about the es-discuss mailing list