bake in reference-counting semantics to idpl Alloc/Dealloc methods

NEW
Unassigned

Status

()

Core
IPC
P3
normal
4 years ago
10 months ago

People

(Reporter: froydnj, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

4 years ago
A lot of IPDL-using code winds up using reference counting, and then plays tricks with .forget().take() and similar so that raw pointers wind up getting passed around.

WDYT about forcing, say, AllocPProtocol to return already_AddRefed<PProtocol>?  Not entirely sure what to do about DeallocPProtocol...maybe make managing protocols hold strong references in their managee arrays?  And eliminate DeallocPProtocol entirely?
Flags: needinfo?(bent.mozilla)

Comment 1

4 years ago
I think it kinda sucks that we ever ended up with reference-counted actors, and I would really hate it if we propagated it to more code than absolutely necessary.
(Reporter)

Updated

4 years ago
Summary: bake in reference-counting semantics to idpl Alloc/Dealoc methods → bake in reference-counting semantics to idpl Alloc/Dealloc methods
(Reporter)

Comment 2

4 years ago
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #1)
> I think it kinda sucks that we ever ended up with reference-counted actors,
> and I would really hate it if we propagated it to more code than absolutely
> necessary.

But why did we end up with reference-counted actors in the first place?  Because the lifetimes of their parent (non-ipdl, presumably reference-counted) entities were a poor match for the tree-structured ipdl stuff, or something else?
Flags: needinfo?(benjamin)

Comment 3

4 years ago
All of the initial plugin tree was carefully not-reference-counted.

When we started hacking in content processes, we started bolting IPDL stuff on the side of existing XPCOM API layers (in particular the necko streams) and rather than having separate IPDL and XPCOM objects, they got unified as a single C++ object which was for both IPDL and XPCOM.

I think that's technical debt we used for initial expediency but something we ought to solve long-term.
Flags: needinfo?(benjamin)
I agree with Benjamin here.
Flags: needinfo?(bent.mozilla)
The pattern I ended up with to avoid ref-counted actors in my code was:

  1) Create pure virtual listener class that provides the ipdl Recv*() and ActorDestroyed() methods.
  2) My ref-counted class implements listener class and calls SetListener() on Actor.
  3) In the listener ActorDestroy() my ref-counted class calls ClearListener() on Actor.
  4) Actor proxies all Recv*() and ActorDestry() calls to listener.

It would be nice if IPDL could generate the code for this kind of listener proxying automatically.  I think it would make it easier to integrate ref-counted classes with actors.

Updated

10 months ago
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.