IPDL: Implement DeallocShmem() and clean up open segments on shutdown

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: cjones, Assigned: cjones)

Tracking

unspecified
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 alpha4+)

Details

Attachments

(2 attachments)

Shmem segments currently just leak.  We need to add a DeallocShmem() interface for closing them at any time, and we also need to free still-open segments during DeallocSubtree().
Bug 548437 refactors shmem a bit and should land on m-c soon, don't want to merge.
Depends on: 548437
Assignee: nobody → jones.chris.g
Attachment #439187 - Flags: review?(bent.mozilla)
Blocks: 560941
bent, this really needs to block the next devpreview, can you get to it?
blocking2.0: --- → alpha4+
Comment on attachment 439187 [details] [diff] [review]
Implement a DeallocShmem() interface

>+Shmem::UnshareFrom(IHadBetterBeIPDLCodeCallingThis_OtherwiseIAmADoodyhead,

I know this has precendent but I feel weird r+'ing something with this in there...
Attachment #439187 - Flags: review?(bent.mozilla) → review+
Comment on attachment 439188 [details] [diff] [review]
Clean up shared memory segments when the top-level actor dies

>diff --git a/ipc/chromium/src/base/id_map.h b/ipc/chromium/src/base/id_map.h
>--- a/ipc/chromium/src/base/id_map.h
>+++ b/ipc/chromium/src/base/id_map.h
>@@ -67,16 +67,22 @@ class IDMap {
>     }
>     data_.erase(i);
>   }
> 
>   bool IsEmpty() const {
>     return data_.empty();
>   }
> 
>+#if defined(CHROMIUM_MOZILLA_BUILD)
>+  void Clear() {
>+    data_.clear();
>+  }
>+#endif
>+
>   T* Lookup(int32 id) const {
>     const_iterator i = data_.find(id);
>     if (i == data_.end())
>       return NULL;
>     return i->second;
>   }
> 
>   size_t size() const {
>diff --git a/ipc/ipdl/ipdl/lower.py b/ipc/ipdl/ipdl/lower.py
>--- a/ipc/ipdl/ipdl/lower.py
>+++ b/ipc/ipdl/ipdl/lower.py
>@@ -1467,16 +1467,25 @@ class Protocol(ipdl.ast.Protocol):
>     def managerArrayExpr(self, thisvar, side):
>         """The member var my manager keeps of actors of my type."""
>         assert self.decl.type.isManaged()
>         return ExprSelect(
>             ExprCall(self.managerMethod(thisvar)),
>             '->', 'mManaged'+ _actorName(self.decl.type.name(), side))
> 
>     # shmem stuff
>+    def shmemMapType(self):
>+        assert self.decl.type.isToplevel()
>+        return Type('IDMap', T=_rawShmemType())
>+
>+    def shmemIteratorType(self):
>+        assert self.decl.type.isToplevel()
>+        # XXX breaks abstractions
>+        return Type('IDMap<SharedMemory>::const_iterator')
>+
>     def shmemMapVar(self):
>         assert self.decl.type.isToplevel()
>         return ExprVar('mShmemMap')
> 
>     def lastShmemIdVar(self):
>         assert self.decl.type.isToplevel()
>         return ExprVar('mLastShmemId')
> 
>@@ -2874,16 +2883,17 @@ class _GenerateProtocolActorCode(ipdl.as
>                 self.cls.addstmts([
>                     makeHandlerMethod('OnCallReceived', self.rpcSwitch,
>                                       hasReply=1, dispatches=dispatches),
>                     Whitespace.NL
>                 ])
> 
>         destroysubtreevar = ExprVar('DestroySubtree')
>         deallocsubtreevar = ExprVar('DeallocSubtree')
>+        deallocshmemvar = ExprVar('DeallocShmems')
> 
>         # OnReplyTimeout()
>         if toplevel.talksSync() or toplevel.talksRpc():
>             ontimeout = MethodDefn(
>                 MethodDecl('OnReplyTimeout', ret=Type.BOOL))
> 
>             if ptype.isToplevel():
>                 ontimeout.addstmt(StmtReturn(
>@@ -2915,30 +2925,32 @@ class _GenerateProtocolActorCode(ipdl.as
>             self.cls.addstmts([ onentered, onexited, onstack, Whitespace.NL ])
> 
>         # OnChannelClose()
>         onclose = MethodDefn(MethodDecl('OnChannelClose'))
>         if ptype.isToplevel():
>             onclose.addstmts([
>                 StmtExpr(ExprCall(destroysubtreevar,
>                                   args=[ _DestroyReason.NormalShutdown ])),
>-                StmtExpr(ExprCall(deallocsubtreevar))
>+                StmtExpr(ExprCall(deallocsubtreevar)),
>+                StmtExpr(ExprCall(deallocshmemvar))
>             ])
>         else:
>             onclose.addstmt(
>                 _runtimeAbort("`OnClose' called on non-toplevel actor"))
>         self.cls.addstmts([ onclose, Whitespace.NL ])
> 
>         # OnChannelError()
>         onerror = MethodDefn(MethodDecl('OnChannelError'))
>         if ptype.isToplevel():
>             onerror.addstmts([
>                 StmtExpr(ExprCall(destroysubtreevar,
>                                   args=[ _DestroyReason.AbnormalShutdown ])),
>-                StmtExpr(ExprCall(deallocsubtreevar))
>+                StmtExpr(ExprCall(deallocsubtreevar)),
>+                StmtExpr(ExprCall(deallocshmemvar))
>             ])
>         else:
>             onerror.addstmt(
>                 _runtimeAbort("`OnError' called on non-toplevel actor"))
>         self.cls.addstmts([ onerror, Whitespace.NL ])
> 
>         # FIXME/bug 535053: only manager protocols and non-manager
>         # protocols with union types need Lookup().  we'll give it to
>@@ -3118,17 +3130,40 @@ class _GenerateProtocolActorCode(ipdl.as
>                 foreachdealloc,
>                 StmtExpr(_callCxxArrayClear(p.managedVar(managed, self.side))),
> 
>             ])
>             deallocsubtree.addstmt(block)
>         # don't delete outselves: either the manager will do it, or
>         # we're toplevel
>         self.cls.addstmts([ deallocsubtree, Whitespace.NL ])
>-        
>+
>+        if ptype.isToplevel():
>+            ## DeallocShmem():
>+            #    for (cit = map.begin(); cit != map.end(); ++cit)
>+            #      Dealloc(cit->second)
>+            #    map.Clear()
>+            deallocshmem = MethodDefn(MethodDecl(deallocshmemvar.name))
>+
>+            citvar = ExprVar('cit')
>+            begin = ExprCall(ExprSelect(p.shmemMapVar(), '.', 'begin'))
>+            end = ExprCall(ExprSelect(p.shmemMapVar(), '.', 'end'))
>+            shmem = ExprSelect(citvar, '->', 'second')
>+            foreachdealloc = StmtFor(
>+                Param(p.shmemIteratorType(), citvar.name, begin),
>+                ExprBinary(citvar, '!=', end),
>+                ExprPrefixUnop(citvar, '++'))
>+            foreachdealloc.addstmt(StmtExpr(_shmemDealloc(shmem)))
>+
>+            deallocshmem.addstmts([
>+                foreachdealloc,
>+                StmtExpr(ExprCall(ExprSelect(p.shmemMapVar(), '.', 'Clear')))
>+            ])
>+            self.cls.addstmts([ deallocshmem, Whitespace.NL ])
>+
>         ## private members
>         self.cls.addstmt(StmtDecl(Decl(p.channelType(), 'mChannel')))
>         if ptype.isToplevel():
>             self.cls.addstmts([
>                 StmtDecl(Decl(Type('IDMap', T=Type('ChannelListener')),
>                               p.actorMapVar().name)),
>                 StmtDecl(Decl(_actorIdType(), p.lastActorIdVar().name)),
>                 StmtDecl(Decl(Type('ProcessHandle'),
>@@ -3137,18 +3172,17 @@ class _GenerateProtocolActorCode(ipdl.as
>         elif ptype.isManaged():
>             self.cls.addstmts([
>                 StmtDecl(Decl(_actorIdType(), p.idVar().name)),
>                 StmtDecl(Decl(p.managerInterfaceType(ptr=1),
>                               p.managerVar().name))
>             ])
>         if p.decl.type.isToplevel():
>             self.cls.addstmts([
>-                StmtDecl(Decl(Type('IDMap', T=_rawShmemType()),
>-                              p.shmemMapVar().name)),
>+                StmtDecl(Decl(p.shmemMapType(), p.shmemMapVar().name)),
>                 StmtDecl(Decl(_shmemIdType(), p.lastShmemIdVar().name))
>             ])
> 
>         for managed in ptype.manages:
>             self.cls.addstmts([
>                 Whitespace('// Sorted by pointer value\n', indent=1),
>                 StmtDecl(Decl(
>                     p.managedVarType(managed, self.side),
Attachment #439188 - Flags: review?(bent.mozilla) → review+
You need to log in before you can comment on or make changes to this bug.