225016 - ping ipcd module

Status: RESOLVED FIXED

(Core :: IPC, enhancement)

Description

[Axel Hecht]

ipc clients should be able to know when the ipcservice established a connection
to the ipcd.
The easiest way I found was to abuse the TestModule (which basically pings).
I'd like to create a module that answers each message it gets with a message
containing just the clientID of the caller.
Creating a dummy service to listen to the category manager and "ipc-startup"
does not work if the ipc is already running.

    gIpcServ->SetMessageObserver(kTestTargetID,
                                 new mfMessageObserver());
    const char hello[] = "hello friend!";
    rv = gIpcServ->SendMessage(0, kTestTargetID, (const PRUint8 *) hello,
                               sizeof(hello), PR_FALSE);
    NS_ENSURE_SUCCESS(rv, 1);

    // process events
    PLEvent *ev;
    while (gKeepRunning) {
        gEventQ->WaitForEvent(&ev);
        gEventQ->HandleEvent(ev);
    }

is the code fragment that one would use to ensure that the connection is 
enabled.

Darin, any preferences where such a module should go? to ipcd/extensions?
That sounds optional, I would like to make the ping-module part of the 
ipcservice contract. (Feel free to assign this bug to me if that suits your 
plans.)

Comment 1

[Darin Fisher]

the built-in IPCM (IPC Manager) target supports a PING message.  we should be
able to use that.  in other words, you should not need to develop a new IPC
daemon extension.

technically, ipcIService::sendMessage should not be used to send/receive IPCM
messages.  in fact, those messages are gobbled up by
ipcService::OnMessageAvailable.  it thinks all IPCM message should be for it.

that means we could add a "PING" method to ipcIService, or we could allow the
PING message to go through ipcService::OnMessageAvailable.  this might be the
simplest thing to do.  but, i'm not entirely sure... let me think it about it
some more.

Comment 2

[Darin Fisher]

ok, here's a plan... since my recent changes to IPC client initialization made
the connect code block until the daemon is up-and-running, we could likewise
have the CLIENT_HELLO message block until it receives a CLIENT_ID message.  in
that way, this bug would be solved.  there simply would be no need for an
asynchronous "we're connected now" notification.  you could just get the IPC
service and ask it for its client ID synchronously.

Comment 3

[Darin Fisher]

Attachment: v1 patch

here we go.  this patch makes ipcTransport::Init until it knows its client ID. 
i think this solution makes the most sense.  i'll assert that there isn't much
cost at all to making this transaction synchronous =)

Comment 4

[Darin Fisher]

axel: can you please give this patch a try, and let me know what you think?  thx!

Comment 5

[Darin Fisher]

Comment on attachment 135021 [details] [diff] [review]
v1 patch

timeless gave r= on irc

Comment 6

[Darin Fisher]

fixed-on-trunk

Comment 7

[Axel Hecht]

What was the removal of the profile change observing good for?

Just curious.

Comment 8

[Darin Fisher]

we needed that code when the daemon connection was handled by necko because
necko forcibly drops all network connections when a profile change occurs.  but,
the ipc daemon doesn't need to be restarted or changed in any way when a profile
change occurs.  it really doens't need to care.  the transaction manager, which
is built on top of the ipc daemon, can handle multiple profiles at a time.  so,
the code to drop the connection on profile change is no longer needed and is in
my opinion the wrong thing.

Comment 9

[Axel Hecht]

fair enough.