Bug 1729044 - Remove ipdl support for intr r=ipc-reviewers,nika

This removes intr support and updates some stale docs as well.

It's not immediately clear what code in MessageChannel can be removed,
though I expect some things could be simplified (there's just not much
alluding to intr/rpc/urgent).

Differential Revision: https://phabricator.services.mozilla.com/D204813
This commit is contained in:
Alex 2024-03-18 17:23:26 +00:00
parent 6c8a0020a5
commit 28e7b97eeb
7 changed files with 22 additions and 127 deletions

View file

@ -73,8 +73,8 @@ order is well defined since the related actor can only send from one thread.
.. warning::
There are a few (rare) exceptions to the message order guarantee. They
include `synchronous nested`_ messages, `interrupt`_ messages, and
messages with a ``[Priority]`` or ``[Compress]`` annotation.
include `synchronous nested`_ messages and messages with a ``[Priority]``
or ``[Compress]`` annotation.
An IPDL protocol file specifies the messages that may be sent between parent
and child actors, as well as the direction and payload of those messages.
@ -85,13 +85,10 @@ running in the actor's thread's ``MessageLoop``.
.. note::
Not all IPDL messages are asynchronous. Again, we run into exceptions for
messages that are synchronous, `synchronous nested`_ or `interrupt`_. Use
of synchronous and nested messages is strongly discouraged but may not
always be avoidable. They will be defined later, along with superior
alternatives to both that should work in nearly all cases. Interrupt
messages were prone to misuse and are deprecated, with removal expected in
the near future
(`Bug 1729044 <https://bugzilla.mozilla.org/show_bug.cgi?id=1729044>`_).
messages that are synchronous or `synchronous nested`_. Use of synchronous
and nested messages is strongly discouraged but may not always be avoidable.
They will be defined later, along with superior alternatives to both that
should work in nearly all cases.
Protocol files are compiled by the *IPDL compiler* in an early stage of the
build process. The compiler generates C++ code that reflects the protocol.
@ -120,7 +117,6 @@ and `here
``chromium-config.mozbuild`` in its ``moz.build`` file. See `Using The
IPDL compiler`_ for a complete list of required build changes.
.. _interrupt: `The Old Ways`_
.. _synchronous nested: `The Rest`_
The Steps To Making A New Actor
@ -386,10 +382,6 @@ the options for the actor-blocking policy of messages:
``sync`` Actor has ``async`` capabilities and adds ``sync``
messages. ``sync`` messages
can only be sent from the child actor to the parent.
``intr`` (deprecated) Actor has ``sync`` capabilities and adds ``intr``
messages. Some messages can be received while an actor
waits for an ``intr`` response. This type will be
removed soon.
======================= =======================================================
Beyond these protocol blocking strategies, IPDL supports annotations that
@ -538,8 +530,6 @@ resembles the list in `Defining Actors`_:
``sync`` Actor has ``async`` capabilities and adds ``sync``
messages. ``sync`` messages can only be sent from the
child actor to the parent.
``intr`` (deprecated) Actor has ``sync`` capabilities and adds ``intr``
messages. This type will be removed soon.
====================== ========================================================
The policy defines whether an actor will wait for a response when it sends a
@ -653,18 +643,13 @@ for use in IPDL files:
``sync/async`` These are used in two cases: (1) to indicate
whether a message blocks as it waits for a result
and (2) because an actor that contains ``sync``
messages must itself be labeled ``sync`` or
``intr``.
messages must itself be labeled ``sync``.
``[NestedUpTo=inside_sync]`` Indicates that an actor contains
[Nested=inside_sync] messages, in addition to
normal messages.
``[NestedUpTo=inside_cpow]`` Indicates that an actor contains
[Nested=inside_cpow] messages, in addition to
normal messages.
``intr`` Used to indicate either that (1) an actor
contains ``sync``, ``async`` and (deprecated)
``intr`` messages, or (2) a message is ``intr``
type.
``[Nested=inside_sync]`` Indicates that the message can be handled while
waiting for lower-priority, or in-message-thread,
sync responses.

View file

@ -110,9 +110,6 @@ struct ActorHandle {
int mId;
};
// What happens if Interrupt calls race?
enum RacyInterruptPolicy { RIPError, RIPChildWins, RIPParentWins };
enum class LinkStatus : uint8_t {
// The actor has not established a link yet, or the actor is no longer in use
// by IPC, and its 'Dealloc' method has been called or is being called.
@ -128,7 +125,7 @@ enum class LinkStatus : uint8_t {
Connected,
// The link has begun being destroyed. Messages may still be received, but
// cannot be sent. (exception: sync/intr replies may be sent while Doomed).
// cannot be sent. (exception: sync replies may be sent while Doomed).
Doomed,
// The link has been destroyed, and messages will no longer be sent or
@ -223,8 +220,6 @@ class IProtocol : public HasResultCodes {
virtual Result OnMessageReceived(const Message& aMessage) = 0;
virtual Result OnMessageReceived(const Message& aMessage,
UniquePtr<Message>& aReply) = 0;
virtual Result OnCallReceived(const Message& aMessage,
UniquePtr<Message>& aReply) = 0;
bool AllocShmem(size_t aSize, Shmem* aOutMem);
bool AllocUnsafeShmem(size_t aSize, Shmem* aOutMem);
bool DeallocShmem(Shmem& aMem);
@ -461,8 +456,7 @@ class IToplevelProtocol : public IRefCountedProtocol {
// the same thread. This method should be called on the thread to perform
// the link.
//
// WARNING: Attempting to send a sync or intr message on the same thread
// will crash.
// WARNING: Attempting to send a sync message on the same thread will crash.
bool OpenOnSameThread(IToplevelProtocol* aTarget,
mozilla::ipc::Side aSide = mozilla::ipc::UnknownSide);

View file

@ -266,10 +266,6 @@ class ASYNC(metaclass=PrettyPrinted):
pretty = "async"
class INTR(metaclass=PrettyPrinted):
pretty = "intr"
class SYNC(metaclass=PrettyPrinted):
pretty = "sync"

View file

@ -218,15 +218,11 @@ def _putInNamespaces(cxxthing, namespaces):
def _sendPrefix(msgtype):
"""Prefix of the name of the C++ method that sends |msgtype|."""
if msgtype.isInterrupt():
return "Call"
return "Send"
def _recvPrefix(msgtype):
"""Prefix of the name of the C++ method that handles |msgtype|."""
if msgtype.isInterrupt():
return "Answer"
return "Recv"
@ -1856,16 +1852,6 @@ def _generateMessageConstructor(md, segmentSize, protocol, forReply=False):
else:
syncEnum = "ASYNC"
# FIXME(bug ???) - remove support for interrupt messages from the IPDL compiler.
if md.decl.type.isInterrupt():
func.addcode(
"""
static_assert(
false,
"runtime support for intr messages has been removed from IPDL");
"""
)
if md.decl.type.isCtor():
ctorEnum = "CONSTRUCTOR"
else:
@ -4033,7 +4019,7 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
for managed in ptype.manages:
self.genManagedEndpoint(managed)
# OnMessageReceived()/OnCallReceived()
# OnMessageReceived()
# save these away for use in message handler case stmts
msgvar = ExprVar("msg__")
@ -4051,11 +4037,8 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
msgtype = ExprCode("msg__.type()")
self.asyncSwitch = StmtSwitch(msgtype)
self.syncSwitch = None
self.interruptSwitch = None
if toplevel.isSync() or toplevel.isInterrupt():
if toplevel.isSync():
self.syncSwitch = StmtSwitch(msgtype)
if toplevel.isInterrupt():
self.interruptSwitch = StmtSwitch(msgtype)
# Add a handler for the MANAGED_ENDPOINT_BOUND and
# MANAGED_ENDPOINT_DROPPED message types for managed actors.
@ -4103,10 +4086,8 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
"""
)
self.asyncSwitch.addcase(DefaultLabel(), default)
if toplevel.isSync() or toplevel.isInterrupt():
if toplevel.isSync():
self.syncSwitch.addcase(DefaultLabel(), default)
if toplevel.isInterrupt():
self.interruptSwitch.addcase(DefaultLabel(), default)
self.cls.addstmts(self.implementManagerIface())
@ -4197,17 +4178,6 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
Whitespace.NL,
]
)
self.cls.addstmts(
[
makeHandlerMethod(
"OnCallReceived",
self.interruptSwitch,
hasReply=True,
dispatches=dispatches,
),
Whitespace.NL,
]
)
clearsubtreevar = ExprVar("ClearSubtree")
@ -4244,22 +4214,6 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
)
self.cls.addstmts([onerror, Whitespace.NL])
if ptype.isToplevel() and ptype.isInterrupt():
processnative = MethodDefn(
MethodDecl("ProcessNativeEventsInInterruptCall", ret=Type.VOID)
)
processnative.addcode(
"""
#ifdef XP_WIN
GetIPCChannel()->ProcessNativeEventsInInterruptCall();
#else
FatalError("This method is Windows-only");
#endif
"""
)
self.cls.addstmts([processnative, Whitespace.NL])
# private methods
self.cls.addstmt(Label.PRIVATE)
@ -4566,8 +4520,6 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
self.asyncSwitch.addcase(lbl, case)
elif decltype.isSync():
self.syncSwitch.addcase(lbl, case)
elif decltype.isInterrupt():
self.interruptSwitch.addcase(lbl, case)
else:
assert 0
@ -5417,8 +5369,6 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
def sendBlocking(self, md, msgexpr, replyexpr, actor=None):
send = ExprVar("ChannelSend")
if md.decl.type.isInterrupt():
send = ExprVar("ChannelCall")
if actor is not None:
send = ExprSelect(actor, "->", send.name)

View file

@ -126,7 +126,6 @@ reserved = set(
"class",
"from",
"include",
"intr",
"manager",
"manages",
"namespace",
@ -552,15 +551,12 @@ def p_AttributeValue(p):
def p_SendSemantics(p):
"""SendSemantics : ASYNC
| SYNC
| INTR"""
| SYNC"""
if p[1] == "async":
p[0] = ASYNC
elif p[1] == "sync":
p[0] = SYNC
else:
assert p[1] == "intr"
p[0] = INTR
assert p[1] == "sync"
p[0] = SYNC
def p_OptionalSendSemantics(p):

View file

@ -8,7 +8,7 @@ import sys
from ipdl.ast import CxxInclude, Decl, Loc, QualifiedId, StructDecl
from ipdl.ast import UnionDecl, UsingStmt, Visitor, StringLiteral
from ipdl.ast import ASYNC, SYNC, INTR
from ipdl.ast import ASYNC, SYNC
from ipdl.ast import IN, OUT, INOUT
from ipdl.ast import NOT_NESTED, INSIDE_SYNC_NESTED, INSIDE_CPOW_NESTED
from ipdl.ast import priorityList
@ -300,9 +300,6 @@ class SendSemanticsType(IPDLType):
def isSync(self):
return self.sendSemantics == SYNC
def isInterrupt(self):
return self.sendSemantics is INTR
def sendSemanticsSatisfiedBy(self, greater):
def _unwrap(nr):
if isinstance(nr, dict):
@ -322,17 +319,10 @@ class SendSemanticsType(IPDLType):
if lnr0 < gnr0 or lnr1 > gnr1:
return False
# Protocols that use intr semantics are not allowed to use
# message nesting.
if greater.isInterrupt() and lesser.nestedRange != (NOT_NESTED, NOT_NESTED):
return False
if lesser.isAsync():
return True
elif lesser.isSync() and not greater.isAsync():
return True
elif greater.isInterrupt():
return True
return False
@ -391,7 +381,7 @@ class MessageType(SendSemanticsType):
return self.direction is INOUT
def hasReply(self):
return len(self.returns) or self.isSync() or self.isInterrupt()
return len(self.returns) or self.isSync()
def hasImplicitActorParam(self):
return self.isCtor()
@ -1372,25 +1362,11 @@ class GatherDecls(TcheckVisitor):
"Priority": priorityList,
"ReplyPriority": priorityList,
"Nested": ("not", "inside_sync", "inside_cpow"),
"LegacyIntr": None,
"VirtualSendImpl": None,
"LazySend": None,
},
)
if md.sendSemantics is INTR and "LegacyIntr" not in md.attributes:
self.error(
loc,
"intr message `%s' allowed only with [LegacyIntr]; DO NOT USE IN SHIPPING CODE",
msgname,
)
if md.sendSemantics is INTR and "Priority" in md.attributes:
self.error(loc, "intr message `%s' cannot specify [Priority]", msgname)
if md.sendSemantics is INTR and "Nested" in md.attributes:
self.error(loc, "intr message `%s' cannot specify [Nested]", msgname)
if md.sendSemantics is not ASYNC and "LazySend" in md.attributes:
self.error(loc, "non-async message `%s' cannot specify [LazySend]", msgname)
@ -1631,11 +1607,6 @@ class CheckTypes(TcheckVisitor):
mgrtype.name(),
)
if ptype.isInterrupt() and ptype.nestedRange != (NOT_NESTED, NOT_NESTED):
self.error(
p.decl.loc, "intr protocol `%s' cannot specify [NestedUpTo]", p.name
)
if ptype.isToplevel():
cycles = checkcycles(p.decl.type)
if cycles:

View file

@ -7,7 +7,10 @@ specifications, and successfully rejecting erroneous specifications.
To run these tests yourself locally, the correct invocation is
make -C obj-dir/ipc/ipdl/test/ipdl check
or
mach build ipc/ipdl/test/ipdl check
The second category (cxx/) is C++ tests of IPDL semantics. These
tests check that async/sync/rpc semantics are implemented correctly,
The second category (gtest/) is C++ tests of IPDL semantics. These
tests check that async/sync semantics are implemented correctly,
ctors/dtors behave as they should, etc.