1273647 - Elucidate reference counting of SharedImmutableStringsCache

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Enlucidate reference counting of SharedImmutableStringsCache

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a9b0aef4fed1

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Elucidate reference counting of SharedImmutableStringsCache

s/enlucidate/elucidate/

Too much emplacing...

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Elucidate reference counting of SharedImmutableStringsCache

Forgot to remove one set of now-replaced refcount++s.

Comment 5

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

New try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d470e4bd8a9

Comment 6

[Jim Blandy :jimb]

Comment on attachment 8753529 [details] [diff] [review]
Elucidate reference counting of SharedImmutableStringsCache

Review of attachment 8753529 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/vm/SharedImmutableStringsCache.cpp
@@ +75,5 @@
>      box_->refcount--;
>      if (box_->refcount == 0)
>          box_->chars_.reset(nullptr);
> +
> +    box_ = nullptr;

How can this matter? Aren't we in the destructor?

::: js/src/vm/SharedImmutableStringsCache.h
@@ +328,5 @@
>          size_t refcount;
>          Set set;
>  
>          Inner()
> +          : refcount(0)

This especially.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/12bbb42dad82

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/12bbb42dad82