On 08/16/2011 01:27 AM, Amos Jeffries wrote: > 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.
>> 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. I would try allocating the global dynamically when first needed instead of statically. This will avoid both polluting inlines and static initialization ordering problems. >> 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? Sorry, I probably missed this documentation when looking at the original patch. The above quotes looks OK to me. > ... and I think that wording should go into the Subscription API > template documentation. Since its always true for Subscriptions. I do not think IPC-specific things should go into Subscription API documentation and some [future] Subscription applications could be specific to receiving a subset of messages, I guess. Thank you, Alex.
