On 16/08/11 10:02, Alex Rousskov wrote:
On 08/13/2011 06:47 AM, Amos Jeffries wrote:
On 13/08/11 05:23, Amos Jeffries wrote:
This adds support for components to register to receive AsyncCalls when
IPC messages are received.

The subscription is linked to a message type. With currently one
registered recipient Job per type.


Updated version. Includes a new file ipc/Messages.cc to define the
subscriptions registry global.

Now tested and works.

Can you hide Ipc::MessageSubscriptions from subscribers? It seems like
that variable does not need to be globally visible, and there is no need
to inline registration management.

Not in its current form. It is used by Ipc::Coordinator internals. The inlining prevents initialization issues with library internal globals.

Tried making them members of Ipc::Coordinator to get around both those problems. Should work but I'm hitting an assert now on the Ipc::Coordinator::receive dynamic cast.


Consider relaxing the RegisterListenFor() check to allow repeated
registrations if callSub has not changed. After all, you do allow
repeated unregistrations.

Okay. Allowing avoidance of the assert. But documenting as unique registrations required, so we don't get any funny attempts at abuse.


It may be worth documenting that registration API is for listening to
_all_ messages of a certain type, and is not suitable for catching an
individual response to a specific IPC request.

AKA. "subscribe a callback to happen whenever a certain message class arrives"
 ... no mention of termination (until un-registered)
 ... or numbers (indeterminate, even if unregistered on first receipt).

ClearListen for states "already scheduled events may exist and will happen". Plural on the events queued at de-registration time.

Seems pretty unambiguous that this is starting and stopping a stream of "things", rather than discrete reads. This is true of all subscriptions. Even the FTP data listener is written to wade through N data channel attacks trying to find the one which was the actual server.

How else could this be worded?

... and I think that wording should go into the Subscription API template documentation. Since its always true for Subscriptions.


The caller is supposed to
write a second layer mapping/registration routine that will map
individual responses to the code/state waiting for them.


see ipc/Messages.h
"
 * Subscribers must create a Dialer and a AsyncCall to receive
TypedMsgHdr parameters
 * then generate a new CallSubscription<Dialer>(AsyncCall).
* then use RegisterListenFor(message type, subscription) to start receiving IPC messages
"

There is no requirement on what the receiver may do when getting a TypedMsgHdr. Might translate to some other type. Might use TypedMsgHdr directly. Might do nothing. These patches are not changing the existing limits on IPC messages.


Fixed spelling and changed that to say "const TypedMsgHdr" though, so its clear they must not write to it.

Amos
--
Please be using
  Current Stable Squid 2.7.STABLE9 or 3.1.14
  Beta testers wanted for 3.2.0.10

Reply via email to