stylo: Add refcount logging for servo_arc so we can detect reference cycles that cause leaks

NEW
Unassigned

Status

()

enhancement
P3
normal
2 years ago
2 years ago

People

(Reporter: bzbarsky, Unassigned)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 wontfix)

Details

Gecko has the ability to log addref/release, complete with stacks and whatnot, to enable detection of leaks of refcounted objects.

We don't seem to have anything like that for servo_arc.

Note that due to Rust's semantics for Arc I'm not terribly worried about leaks due to manually unbalanced release/addref, because those don't happen manually.  What I _am_ worried about is leaks due to reference cycles.  Right now I don't believe we have any real visibility into this, right?
Summary: stylo: Add refcount logging for servo_arc → stylo: Add refcount logging for servo_arc so we can detect reference cycles that cause leaks
(In reply to Boris Zbarsky [:bz] (still digging out from vacation mail) from comment #0)
> Gecko has the ability to log addref/release, complete with stacks and
> whatnot, to enable detection of leaks of refcounted objects.
> 
> We don't seem to have anything like that for servo_arc.
> 
> Note that due to Rust's semantics for Arc I'm not terribly worried about
> leaks due to manually unbalanced release/addref, because those don't happen
> manually.  What I _am_ worried about is leaks due to reference cycles. 
> Right now I don't believe we have any real visibility into this, right?

Well, anything that does MOZ_COUNT_CTOR/MOZ_COUNT_DTOR will be fine, right? That's certainly the case for style contexts and style structs.
> Well, anything that does MOZ_COUNT_CTOR/MOZ_COUNT_DTOR will be fine, right?

Yes, that's true.

> That's certainly the case for style contexts and style structs.

I'm not terribly worried about style contexts and style structs, because they should really not have any cyclical anything going on.  The fact that we do lock their construction/destruction does help.  I was a bit more worried about the data structures the CSS parser produces, say.
Priority: -- → P3
I guess we can reuse Gecko's NS_LogCtor and NS_LogDtor on Arc::new and Arc::drop_slow respectively.

There are three blockers, though:
1. We cannot get type name string from Rust. Rust has an intrinsic "type_name" which returns a string representing the type, but intrinsics are unstable, and there is no stable interface exposed for that.
2. Even if we can get the type string somehow, the Gecko functions want a C-style string, which Rust isn't going to give us anyway, so we may need to either duplicate some of the code, or teach the existing code to accept slice.
3. Arc supports unsized types, but GetBloatEntry asserts that type with the same name has the same size. This would only be a problem if we use Arc with slice or str, I guess, and IIRC we don't, so it's probably not a big issue.
(This sounds like something we might be able to uplift without too much risk, so let's call this "fix-optional" for 57.)
Version: 53 Branch → Trunk
status-firefox57=wontfix unless someone thinks this bug should block 57
You need to log in before you can comment on or make changes to this bug.