Questioning WeakMap.prototype.clear (was: Private symbols as WeakMap sugar)

Mark S. Miller erights at google.com
Mon Jan 21 10:46:27 PST 2013


On Mon, Jan 21, 2013 at 10:40 AM, Mark S. Miller <erights at google.com> wrote:
> On Mon, Jan 21, 2013 at 9:02 AM, Allen Wirfs-Brock
> <allen at wirfs-brock.com> wrote:
>> If you don't want to expose clear on a WeakMap, create a subclass that doesn't support it:
>>
>> class WeakMapWithoutClear extends WeakMap {
>>    delete() {throw Error("Clearing this map is not allowed");
>> }
>
> Did you mean "clear()" rather than "delete()" ? Assuming so, how does
> this defend against


The above fragment was a leftover from a partial edit, sorry. I had
written your clear.call attack before seeing that you'd already
covered that below and suggested the wrapping defense.


>
>
>
> In any case, your argument cuts both ways. Given WeakMaps without .clear,
>
> // note: implements the WeakMap API but does *not* extend WeakMap.
> class WeakMapWithClear {
>      // doesn't matter whether private is implemented using WeakMap or
> private symbols
>      // as long as it provides real encapsulation.
>     private let wrapped; // not const, however we say this for a field
>     constructor() {
>         wrapped = new WeakMap();
>     }
>     get(key) => wrapped.get(key),
>     set(key, val) => wrapped.set(key, value),
>     has(key) => wrapped.has(key),
>     delete(key) => wrapped.delete(key),
>     clear() => { wrapped = null; }
> }
>
>
> Btw, as a separate point, notice that the above code becomes insecure
> if .set and .delete implicitly chain, unless we wrap curlies around
> the body. Not a strong point by itself because it's not hard to wrap
> curlies around the body, but does show that chaining creates a hazard
> that is easy to trip on.
>
>
>
>>
>> if you are really paranoid that somebody is going to do
>>        WeakMap.prototype.clear.call(myWeakMapWithoutClear);
>> then don't expose myWeakMapWithoutClear to untrusted parties or only expose wrapper object that hides the WeakMap as private state.
>
> Please don't speak out of both sides of your mouth in this way. First
> you show a piece of code as a supposed solution to the problem you
> raise. Then in an aside, you admit your solution is broken but do not
> show the code needed to actually fix it, leading to an underestimation
> of costs. My code above shows the cost of wrapping to get the dual
> effect. It's pay as you go -- the built-in weakmaps do not need to pay
> the costs of .clear. Note that in the reversed weakmap representation
> we're talking about, which will be common, these costs are real.
>
>
>>
>> clear is an operation that can not be otherwise synthesized for WeakMaps.
>
> See counter example above.
>
>>  Given the ambient impact of the mere existance of WeakMap entries on garbage collection algorithms, it seems likely that  will be performance advantages in some situations to proactively clear a WeakMap rather than waiting for the GC to do so.  Having clear enables this while it doesn't prevent using WeakMaps in ways that don't expose that operation to ambient use.
>
> This argument is backwards. It is the presence of .clear that prevents
> the non-ephemeron optimization for the common rights-amplification
> pattern.
>
>
>
>>
>>  On Jan 21, 2013, at 3:04 AM, David Bruant wrote:
>>
>>> Le 21/01/2013 11:58, David Bruant a écrit :
>>>> Before the clear method, weakmaps had the following property: "you can only modify a weakmap entry if you have the key". This property isn't true anymore with .clear. This may be considered as abusive ambient authority.
>>> Let's see how this feature came to appear:
>>> Jason Orendorff talked about Map.prototype.clear [1] (Oct 22nd). Seen as a good idea. No discussion on whether it's a good idea for WeakMaps specifically. Nicholas Zakas briefly mentions it in November [2]. No one replied to it specifically. I haven't seen any discussion about it in meeting notes [3]. A brief mention of Set/Map.prototype.clear [4] as a review of the Oct 26th draft [5] (note, 4 days after Jason post, which is a very short amount of time) but nothing about WeakMap.prototype.clear. Implemented in Firefox soon after [6]...
>>> I think WeakMap.prototype.clear slipped through the crack without being specifically discussed. Based on what's publicly available, I don't see anyone noticed and discussed the fact that WeakMap.prototype.clear questions the property that was true before its adoption ("you can only modify a weakmap entry if you have the key")
>>
>> As much as makes sense we want Map, WeakMap, and Set to expose the same interfaces.  Any new method added to one is going to get added to all of them, unless it either simply makes no sense for one of them or if somebody can make a convincing argument for excluding it for one of them.
>>
>>>
>>> I think the property I mentioned is cricial to weakmap integrity and I think WeakMap.prototype.clear should be considered for removal... or at least proper discussion since none really happened from what I've found.
>>
>> TC39 doesn't have a process that requires a priori discussion on es-discuss of every design detailed before it can become part of specification draft.  I'm pretty sure we would never finish anything it that was the case. The spec. drafts exist so the actual details of the proposed specification an be reviewed and discussed like this.
>>
>> Allen
>>
>>
>>
>>
>>>
>>> David
>>>
>>> [1] https://mail.mozilla.org/pipermail/es-discuss/2012-October/025962.html
>>> [2] https://mail.mozilla.org/pipermail/es-discuss/2012-November/026114.html
>>> [3] https://github.com/rwldrn/tc39-notes
>>> [4] https://github.com/rwldrn/tc39-notes/blob/master/es6/2012-11/nov-27.md#review-of-new-draft-spec
>>> [5] http://wiki.ecmascript.org/doku.php?id=harmony:specification_drafts
>>> [6] https://bugzilla.mozilla.org/show_bug.cgi?id=814562
>>> _______________________________________________
>>> 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
>
>
>
> --
>     Cheers,
>     --MarkM



--
    Cheers,
    --MarkM


More information about the es-discuss mailing list