diff --git a/modules/ipc/daemon/Makefile.in b/modules/ipc/daemon/Makefile.in index 1b64b1da3efc..2e072b02f31f 100644 --- a/modules/ipc/daemon/Makefile.in +++ b/modules/ipc/daemon/Makefile.in @@ -79,3 +79,8 @@ LIBS = \ $(NULL) include $(topsrcdir)/config/rules.mk + +# For fruncate +ifeq ($(OS_ARCH),Linux) +DEFINES += -D_BSD_SOURCE +endif diff --git a/modules/ipc/daemon/ipcClient.cpp b/modules/ipc/daemon/ipcClient.cpp index 70b01579a0f1..f9261c0ded40 100644 --- a/modules/ipc/daemon/ipcClient.cpp +++ b/modules/ipc/daemon/ipcClient.cpp @@ -62,6 +62,7 @@ ipcClient::Init() // every client must be able to handle IPCM messages. mTargets.Append(IPCM_TARGET); + // XXX cleanup // see ipcCommandModule for this: //IPC_NotifyClientUp(this); } @@ -142,6 +143,7 @@ ipcClient::DelTarget(const nsID &target) int ipcClient::Process(PRFileDesc *fd, int poll_flags) { + // XXX check if not read OR write if ((poll_flags & PR_POLL_ERR) || (poll_flags & PR_POLL_HUP) || (poll_flags & PR_POLL_EXCEPT) || @@ -159,7 +161,7 @@ ipcClient::Process(PRFileDesc *fd, int poll_flags) if (poll_flags & PR_POLL_READ) { LOG(("client socket is now readable\n")); - char buf[1024]; + char buf[1024]; // XXX 4k? PRInt32 n; // find out how much data is available for reading... @@ -169,11 +171,12 @@ ipcClient::Process(PRFileDesc *fd, int poll_flags) if (n <= 0) return 0; // cancel connection - char *ptr = buf; + const char *ptr = buf; while (n) { PRUint32 nread; PRBool complete; + // XXX check return value mInMsg.ReadFrom(ptr, PRUint32(n), &nread, &complete); if (complete) { @@ -222,6 +225,7 @@ ipcClient::WriteMsgs(PRFileDesc *fd) if (nw == bufLen) mOutMsgQ.DeleteFirst(); + // XXX mSendOffset = 0; else mSendOffset += nw; } diff --git a/modules/ipc/daemon/ipcModule.h b/modules/ipc/daemon/ipcModule.h index 310ea9185bf4..6add0c8e3290 100644 --- a/modules/ipc/daemon/ipcModule.h +++ b/modules/ipc/daemon/ipcModule.h @@ -40,6 +40,7 @@ #include "nsID.h" +// XXX cruft #define IPC_EXPORT extern "C" NS_EXPORT class ipcMessage; @@ -229,7 +230,7 @@ struct ipcModuleEntry }; // -// IPC_EXPORT int IPC_GetModules(ipcDaemonMethods *, ipcModuleEntry **); +// IPC_EXPORT int IPC_GetModules(const ipcDaemonMethods *, const ipcModuleEntry **); // // params: // methods - the daemon's methods @@ -238,6 +239,6 @@ struct ipcModuleEntry // returns: // length of the |entries| array. // -typedef int (* ipcGetModulesFunc) (ipcDaemonMethods *methods, ipcModuleEntry **entries); +typedef int (* ipcGetModulesFunc) (const ipcDaemonMethods *methods, const ipcModuleEntry **entries); #endif // !ipcModule_h__ diff --git a/modules/ipc/daemon/ipcModuleReg.cpp b/modules/ipc/daemon/ipcModuleReg.cpp index 1fecc45d411c..be31f6baebed 100644 --- a/modules/ipc/daemon/ipcModuleReg.cpp +++ b/modules/ipc/daemon/ipcModuleReg.cpp @@ -40,6 +40,7 @@ #include "prlink.h" #include "prio.h" +#include "prlog.h" #include "plstr.h" #include "ipcConfig.h" @@ -49,6 +50,8 @@ #include "ipcCommandModule.h" #include "ipcd.h" +//----------------------------------------------------------------------------- + struct ipcModuleRegEntry { nsID target; @@ -59,19 +62,32 @@ struct ipcModuleRegEntry #define IPC_MAX_MODULE_COUNT 64 static ipcModuleRegEntry ipcModules[IPC_MAX_MODULE_COUNT]; -static int ipcModuleCount; +static int ipcModuleCount = 0; + +//----------------------------------------------------------------------------- static PRStatus -AddModule(const nsID &target, ipcModuleMethods *methods, PRLibrary *lib) +AddModule(const nsID &target, ipcModuleMethods *methods, const char *libPath) { if (ipcModuleCount == IPC_MAX_MODULE_COUNT) { LOG(("too many modules!\n")); return PR_FAILURE; } + if (!methods) { + PR_NOT_REACHED("null module methods"); + return PR_FAILURE; + } + + // + // each ipcModuleRegEntry holds a reference to a PRLibrary, and on + // shutdown, each PRLibrary reference will be released. this ensures + // that the library will not be unloaded until all of the modules in + // that library are shutdown. + // ipcModules[ipcModuleCount].target = target; ipcModules[ipcModuleCount].methods = methods; - ipcModules[ipcModuleCount].lib = lib; + ipcModules[ipcModuleCount].lib = PR_LoadLibrary(libPath); ++ipcModuleCount; return PR_SUCCESS; @@ -82,7 +98,7 @@ InitModuleFromLib(const char *modulesDir, const char *fileName) { LOG(("InitModuleFromLib [%s]\n", fileName)); - static ipcDaemonMethods gDaemonMethods = + static const ipcDaemonMethods gDaemonMethods = { IPC_DAEMON_METHODS_VERSION, IPC_DispatchMsg, @@ -115,12 +131,13 @@ InitModuleFromLib(const char *modulesDir, const char *fileName) LOG((" func=%p\n", (void*) func)); if (func) { - ipcModuleEntry *entries = NULL; + const ipcModuleEntry *entries = NULL; int count = func(&gDaemonMethods, &entries); for (int i=0; iinit) - entries[i].methods->init(); + if (AddModule(entries[i].target, entries[i].methods, buf)) { + if (entries[i].methods->init) + entries[i].methods->init(); + } } } PR_UnloadLibrary(lib); @@ -156,7 +173,7 @@ IPC_InitModuleReg(const char *exePath) // // register plug-in modules // - static const char relModDir[] = "ipc/modules"; + static const char relModDir[] = "ipc/modules"; // XXX fix slashes char *p = PL_strrchr(exePath, IPC_PATH_SEP_CHAR); if (p == NULL) { @@ -190,6 +207,8 @@ IPC_InitModuleReg(const char *exePath) } PR_CloseDir(dir); } + + free(modulesDir); } void @@ -198,15 +217,13 @@ IPC_ShutdownModuleReg() // // shutdown modules in reverse order // - for (int i = ipcModuleCount - 1; i >= 0; --i) { - ipcModuleRegEntry &entry = ipcModules[i]; + while (ipcModuleCount) { + ipcModuleRegEntry &entry = ipcModules[--ipcModuleCount]; if (entry.methods->shutdown) entry.methods->shutdown(); if (entry.lib) PR_UnloadLibrary(entry.lib); } - // memset(ipcModules, 0, sizeof(ipcModules)); - ipcModuleCount = 0; } void diff --git a/modules/ipc/daemon/ipcModuleReg.h b/modules/ipc/daemon/ipcModuleReg.h index d1d466852770..008a5c9970dd 100644 --- a/modules/ipc/daemon/ipcModuleReg.h +++ b/modules/ipc/daemon/ipcModuleReg.h @@ -41,7 +41,8 @@ #include "ipcModule.h" // -// called to init the module registry. +// called to init the module registry. this may only be called once at +// startup or once after calling IPC_ShutdownModuleReg. // // params: // exePath - path to the daemon executable. modules are loaded from a @@ -50,7 +51,8 @@ void IPC_InitModuleReg(const char *exePath); // -// called to shutdown the module registry. +// called to shutdown the module registry. this may be called more than +// once and need not follow a call to IPC_InitModuleReg. // void IPC_ShutdownModuleReg(); @@ -59,6 +61,8 @@ void IPC_ShutdownModuleReg(); // ipcModuleMethods *IPC_GetModuleByTarget(const nsID &target); +// XXX "handle msg for target" instead + // // notifies all modules of client connect/disconnect // diff --git a/modules/ipc/daemon/ipcModuleUtil.h b/modules/ipc/daemon/ipcModuleUtil.h index 590eac365be1..4febf93d1c6d 100644 --- a/modules/ipc/daemon/ipcModuleUtil.h +++ b/modules/ipc/daemon/ipcModuleUtil.h @@ -41,10 +41,12 @@ #include "prlog.h" #include "ipcModule.h" -extern ipcDaemonMethods *gIPCDaemonMethods; +extern const ipcDaemonMethods *gIPCDaemonMethods; //----------------------------------------------------------------------------- // inline wrapper functions +// +// XXX only usable inside module.. blah //----------------------------------------------------------------------------- inline PRStatus @@ -141,13 +143,14 @@ IPC_SendMsg(PRUint32 clientID, const nsID &target, const void *data, PRUint32 da // module factory macros //----------------------------------------------------------------------------- -#define IPC_IMPL_GETMODULES(_modName, _modEntries) \ - ipcDaemonMethods *gIPCDaemonMethods; \ - IPC_EXPORT int \ - IPC_GetModules(ipcDaemonMethods *dmeths, ipcModuleEntry **ents) { \ - gIPCDaemonMethods = dmeths; \ - *ents = _modEntries; \ - return sizeof(_modEntries) / sizeof(ipcModuleEntry); \ +#define IPC_IMPL_GETMODULES(_modName, _modEntries) \ + const ipcDaemonMethods *gIPCDaemonMethods; \ + IPC_EXPORT int \ + IPC_GetModules(const ipcDaemonMethods *dmeths, const ipcModuleEntry **ents) { \ + /* XXX do version checking */ \ + gIPCDaemonMethods = dmeths; \ + *ents = _modEntries; \ + return sizeof(_modEntries) / sizeof(ipcModuleEntry); \ } #endif // !ipcModuleUtil_h__ diff --git a/modules/ipc/daemon/ipcd.cpp b/modules/ipc/daemon/ipcd.cpp index d04cc91aa6ca..a620afcff85c 100644 --- a/modules/ipc/daemon/ipcd.cpp +++ b/modules/ipc/daemon/ipcd.cpp @@ -48,6 +48,8 @@ PRStatus IPC_DispatchMsg(ipcClient *client, const ipcMessage *msg) { + // XXX assert args valid + // if (msg->Target().Equals(IPCM_TARGET)) { IPCM_HandleMsg(client, msg); return PR_SUCCESS; @@ -84,8 +86,10 @@ PRStatus IPC_DispatchMsg(ipcClient *client, const nsID &target, const void *data, PRUint32 dataLen) { // lookup handler for this message's topic and forward message to it. + // XXX methods should be |const| ipcModuleMethods *methods = IPC_GetModuleByTarget(target); if (methods) { + // XXX make sure handleMsg not null methods->handleMsg(client, target, data, dataLen); return PR_SUCCESS; } @@ -124,6 +128,8 @@ IPC_GetClientByName(const char *name) void IPC_EnumClients(ipcClientEnumFunc func, void *closure) { + // XXX null check + // XXX check return val for STOP for (int i = 0; i < ipcClientCount; ++i) func(closure, &ipcClients[i], ipcClients[i].ID()); } @@ -131,24 +137,29 @@ IPC_EnumClients(ipcClientEnumFunc func, void *closure) PRUint32 IPC_GetClientID(ipcClient *client) { + // XXX null check return client->ID(); } const char * IPC_GetPrimaryClientName(ipcClient *client) { + // XXX null check + // XXX eliminate return client->PrimaryName(); } PRBool IPC_ClientHasName(ipcClient *client, const char *name) { + // XXX null check return client->HasName(name); } PRBool IPC_ClientHasTarget(ipcClient *client, const nsID &target) { + // XXX null check return client->HasTarget(target); } @@ -174,6 +185,7 @@ IPC_EnumClientTargets(ipcClient *client, ipcClientTargetEnumFunc func, void *clo } } +// XXX remove ipcClient * IPC_GetClients(PRUint32 *count) { diff --git a/modules/ipc/daemon/ipcdUnix.cpp b/modules/ipc/daemon/ipcdUnix.cpp index 05b3d82dd390..3d8d10d8c2aa 100644 --- a/modules/ipc/daemon/ipcdUnix.cpp +++ b/modules/ipc/daemon/ipcdUnix.cpp @@ -63,26 +63,29 @@ // ipc directory and locking... //----------------------------------------------------------------------------- -static char *ipcDir; -static char *ipcSockFile; -static char *ipcLockFile; -static int ipcLockFD; +static int ipcLockFD = 0; -static PRBool AcquireDaemonLock() +static PRBool AcquireDaemonLock(const char *baseDir) { const char lockName[] = "lock"; - int dirLen = strlen(ipcDir); - int len = dirLen // ipcDir + int dirLen = strlen(baseDir); + int len = dirLen // baseDir + 1 // "/" + sizeof(lockName); // "lock" - ipcLockFile = (char *) malloc(len); - memcpy(ipcLockFile, ipcDir, dirLen); - ipcLockFile[dirLen] = '/'; - memcpy(ipcLockFile + dirLen + 1, lockName, sizeof(lockName)); + char *lockFile = (char *) malloc(len); + memcpy(lockFile, baseDir, dirLen); + lockFile[dirLen] = '/'; + memcpy(lockFile + dirLen + 1, lockName, sizeof(lockName)); + + // + // open lock file. it remains open until we shutdown. + // + ipcLockFD = open(lockFile, O_WRONLY|O_CREAT, S_IWUSR|S_IRUSR); + + free(lockFile); - ipcLockFD = open(ipcLockFile, O_WRONLY|O_CREAT|O_TRUNC, S_IWUSR|S_IRUSR); if (ipcLockFD == -1) return PR_FALSE; @@ -100,11 +103,16 @@ static PRBool AcquireDaemonLock() if (fcntl(ipcLockFD, F_SETLK, &lock) == -1) return PR_FALSE; + // + // truncate lock file once we have exclusive access to it. + // + ftruncate(ipcLockFD, 0); + // // write our PID into the lock file (this just seems like a good idea... // no real purpose otherwise). // - char buf[256]; + char buf[32]; int nb = PR_snprintf(buf, sizeof(buf), "%u\n", (unsigned long) getpid()); write(ipcLockFD, buf, nb); @@ -115,63 +123,45 @@ static PRBool InitDaemonDir(const char *socketPath) { LOG(("InitDaemonDir [sock=%s]\n", socketPath)); - ipcSockFile = PL_strdup(socketPath); - ipcDir = PL_strdup(socketPath); + char *baseDir = PL_strdup(socketPath); // // make sure IPC directory exists (XXX this should be recursive) // - char *p = strrchr(ipcDir, '/'); + char *p = strrchr(baseDir, '/'); if (p) p[0] = '\0'; - mkdir(ipcDir, 0700); + mkdir(baseDir, 0700); // // if we can't acquire the daemon lock, then another daemon // must be active, so bail. // - if (!AcquireDaemonLock()) - return PR_FALSE; + PRBool haveLock = AcquireDaemonLock(baseDir); - // - // delete an existing socket to prevent bind from failing. - // - unlink(socketPath); + PL_strfree(baseDir); - return PR_TRUE; + if (haveLock) { + // delete an existing socket to prevent bind from failing. + unlink(socketPath); + } + return haveLock; } static void ShutdownDaemonDir() { - LOG(("ShutdownDaemonDir [sock=%s]\n", ipcSockFile)); + LOG(("ShutdownDaemonDir\n")); - // - // unlink files from directory before giving up lock; otherwise, we'd be - // unable to delete it w/o introducing a race condition. - // - unlink(ipcLockFile); - unlink(ipcSockFile); + // deleting directory and files underneath it allows another process + // to think it has exclusive access. better to just leave the hidden + // directory in /tmp and let the OS clean it up via the usual tmpdir + // cleanup cron job. - // - // should be able to remove the ipc directory now. - // - rmdir(ipcDir); - - // // this removes the advisory lock, allowing other processes to acquire it. - // - close(ipcLockFD); - - // - // free allocated memory - // - PL_strfree(ipcDir); - PL_strfree(ipcSockFile); - free(ipcLockFile); - - ipcDir = NULL; - ipcSockFile = NULL; - ipcLockFile = NULL; + if (ipcLockFD) { + close(ipcLockFD); + ipcLockFD = 0; + } } //----------------------------------------------------------------------------- @@ -186,7 +176,7 @@ int ipcClientCount; // // the first element of this array is always zero; this is done so that the -// n'th element of ipcClientArray corresponds to the n'th element of +// k'th element of ipcClientArray corresponds to the k'th element of // ipcPollList. // static ipcClient ipcClientArray[IPC_MAX_CLIENTS + 1]; @@ -250,6 +240,7 @@ static int RemoveClient(int clientIndex) static void PollLoop(PRFileDesc *listenFD) { + // the first element of ipcClientArray is unused. ipcClients = ipcClientArray + 1; ipcClientCount = 0; @@ -312,18 +303,19 @@ static void PollLoop(PRFileDesc *listenFD) clientFD = PR_Accept(listenFD, &clientAddr, PR_INTERVAL_NO_WAIT); if (clientFD == NULL) { + // ignore this error... perhaps the client disconnected. LOG(("PR_Accept failed [%d]\n", PR_GetError())); - return; } + else { + // make socket non-blocking + PRSocketOptionData opt; + opt.option = PR_SockOpt_Nonblocking; + opt.value.non_blocking = PR_TRUE; + PR_SetSocketOption(clientFD, &opt); - // make socket non-blocking - PRSocketOptionData opt; - opt.option = PR_SockOpt_Nonblocking; - opt.value.non_blocking = PR_TRUE; - PR_SetSocketOption(clientFD, &opt); - - if (AddClient(clientFD) != 0) - PR_Close(clientFD); + if (AddClient(clientFD) != 0) + PR_Close(clientFD); + } } } @@ -363,7 +355,7 @@ IPC_PlatformSendMsg(ipcClient *client, ipcMessage *msg) int main(int argc, char **argv) { - PRFileDesc *listenFD; + PRFileDesc *listenFD = NULL; PRNetAddr addr; // @@ -371,6 +363,7 @@ int main(int argc, char **argv) // which spawned this daemon. // signal(SIGINT, SIG_IGN); + // XXX block others? check cartman // ensure strict file permissions umask(0077); @@ -383,12 +376,6 @@ int main(int argc, char **argv) //LOG(("sleeping for 2 seconds...\n")); //PR_Sleep(PR_SecondsToInterval(2)); - listenFD = PR_OpenTCPSocket(PR_AF_LOCAL); - if (!listenFD) { - LOG(("PR_OpenUDPSocket failed [%d]\n", PR_GetError())); - return -1; - } - const char *socket_path; if (argc < 2) socket_path = IPC_DEFAULT_SOCKET_PATH; @@ -396,8 +383,14 @@ int main(int argc, char **argv) socket_path = argv[1]; if (!InitDaemonDir(socket_path)) { - PR_Close(listenFD); - return 0; + LOG(("InitDaemonDir failed\n")); + goto end; + } + + listenFD = PR_OpenTCPSocket(PR_AF_LOCAL); + if (!listenFD) { + LOG(("PR_OpenUDPSocket failed [%d]\n", PR_GetError())); + goto end; } addr.local.family = PR_AF_LOCAL; @@ -405,23 +398,30 @@ int main(int argc, char **argv) if (PR_Bind(listenFD, &addr) != PR_SUCCESS) { LOG(("PR_Bind failed [%d]\n", PR_GetError())); - return -1; + goto end; } IPC_InitModuleReg(argv[0]); if (PR_Listen(listenFD, 5) != PR_SUCCESS) { LOG(("PR_Listen failed [%d]\n", PR_GetError())); - return -1; + goto end; } PollLoop(listenFD); +end: IPC_ShutdownModuleReg(); + // it is critical that we release the lock before closing the socket, + // otherwise, a client might launch another daemon that would be unable + // to acquire the lock and would then leave the client without a daemon. + ShutdownDaemonDir(); - LOG(("closing socket\n")); - PR_Close(listenFD); + if (listenFD) { + LOG(("closing socket\n")); + PR_Close(listenFD); + } return 0; } diff --git a/modules/ipc/daemon/ipcdWin.cpp b/modules/ipc/daemon/ipcdWin.cpp index 582b91095f59..c0ebb5186d32 100644 --- a/modules/ipc/daemon/ipcdWin.cpp +++ b/modules/ipc/daemon/ipcdWin.cpp @@ -71,7 +71,7 @@ RemoveClient(ipcClient *client) int clientIndex = client - ipcClientArray; - client->Finalize(); + client->Finalize(); // XXX pick another name.. // // move last ipcClient object down into the spot occupied by this client. @@ -113,6 +113,8 @@ PurgeStaleClients() IPC_GetClientWindowName(client->PID(), wName); + // XXX dougt has ideas about how to make this better + HWND hwnd = FindWindow(IPC_CLIENT_WINDOW_CLASS, wName); if (!hwnd) { LOG((" client window not found; removing client!\n")); @@ -140,7 +142,7 @@ AddClient(HWND hwnd, PRUint32 pid) ipcClient *client = &ipcClientArray[ipcClientCount]; client->Init(); client->SetHwnd(hwnd); - client->SetPID(pid); + client->SetPID(pid); // XXX one funhction instead of 3 ++ipcClientCount; LOG((" num clients = %u\n", ipcClientCount)); @@ -185,6 +187,7 @@ ProcessMsg(HWND hwnd, PRUint32 pid, const ipcMessage *msg) else client = AddClient(hwnd, pid); + // XXX add logging if (client == NULL) return; @@ -227,6 +230,7 @@ WindowProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam) ipcMessage msg; PRUint32 bytesRead; PRBool complete; + // XXX avoid extra malloc PRStatus rv = msg.ReadFrom((const char *) cd->lpData, cd->cbData, &bytesRead, &complete); if (rv == PR_SUCCESS && complete) { @@ -241,6 +245,7 @@ WindowProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam) return TRUE; } + // XXX don't check wParam if (uMsg == WM_TIMER && wParam == IPC_PURGE_TIMER_ID) { PurgeStaleClients(); return 0; @@ -263,7 +268,7 @@ WindowProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam) } if (uMsg == IPC_WM_SHUTDOWN) { - DestroyWindow(hWnd); + DestroyWindow(hWnd); // XXX possibly move out... think about shutdown sync ipcHwnd = NULL; PostQuitMessage(0); return 0; @@ -288,12 +293,10 @@ AcquireLock() return PR_FALSE; } - if (ipcSyncEvent) { - // check to see if event already existed prior to this call. - if (GetLastError() == ERROR_ALREADY_EXISTS) { - LOG((" lock already set; exiting...\n")); - return PR_FALSE; - } + // check to see if event already existed prior to this call. + if (GetLastError() == ERROR_ALREADY_EXISTS) { + LOG((" lock already set; exiting...\n")); + return PR_FALSE; } LOG((" acquired lock\n")); @@ -323,6 +326,7 @@ main(int argc, char **argv) if (!AcquireLock()) return 0; + // XXX add comments ipcClients = ipcClientArray; ipcClientCount = 0; diff --git a/modules/ipc/public/ipcIService.idl b/modules/ipc/public/ipcIService.idl index 904708a3a8e2..15ffb8064102 100644 --- a/modules/ipc/public/ipcIService.idl +++ b/modules/ipc/public/ipcIService.idl @@ -89,7 +89,7 @@ interface ipcIService : nsISupports * "news", "addrbook", etc. other IPC clients can then query the IPC * daemon for the client named "mail" in order to talk with a mail program. * - * An IPC client name resembles a XPCOM contract ID. + * XXX An IPC client name resembles a XPCOM contract ID. */ void addClientName(in ACString aName); void removeClientName(in ACString aName); @@ -144,6 +144,8 @@ interface ipcIService : nsISupports */ void setClientObserver(in ipcIClientObserver aObserver); + // XXX need other functions to enumerate clients, clients implementing targets, etc. + /** * set a message observer for a particular message target. * @@ -223,15 +225,19 @@ interface ipcIClientObserver : nsISupports interface ipcIClientQueryHandler : nsISupports { /** - * called on successful completion of a client query. + * called on completion of a client query. * * @param aQueryID * the return value from one of the "query" methods. + * @param aStatus + * the status of the query. if this is a failure code, then the + * query failed, otherwise the query succeeded. the value of this + * parameter explains the reason for any failure. * @param aClientID - * * ... */ void onQueryComplete(in unsigned long aQueryID, + in nsresult aStatus, in unsigned long aClientID, [array, size_is(aNameCount)] in string aClientNames, @@ -239,9 +245,6 @@ interface ipcIClientQueryHandler : nsISupports [array, const, size_is(aTargetCount)] in nsIDPtr aClientTargets, in unsigned long aTargetCount); - - void onQueryFailed(in unsigned long aQueryID, - in nsresult aReason); }; %{C++ diff --git a/modules/ipc/src/ipcService.cpp b/modules/ipc/src/ipcService.cpp index 0570347b45d1..f7696fbf84a0 100644 --- a/modules/ipc/src/ipcService.cpp +++ b/modules/ipc/src/ipcService.cpp @@ -43,6 +43,7 @@ #include "nsIObserverService.h" #include "nsICategoryManager.h" #include "nsCategoryManagerUtils.h" +#include "netCore.h" #include "ipcConfig.h" #include "ipcLog.h" @@ -79,12 +80,10 @@ public: static PRUint32 gLastQueryID; void SetClientID(PRUint32 cID) { mClientID = cID; } - - void OnQueryComplete(const ipcmMessageClientInfo *msg); - void OnQueryFailed(nsresult reason); + void OnQueryComplete(nsresult status, const ipcmMessageClientInfo *msg); PRUint32 QueryID() { return mQueryID; } - PRBool IsCanceled() { return mHandler == nsnull; } + PRBool IsCanceled() { return mHandler == NULL; } ipcClientQuery *mNext; private: @@ -96,44 +95,46 @@ private: PRUint32 ipcClientQuery::gLastQueryID = 0; void -ipcClientQuery::OnQueryComplete(const ipcmMessageClientInfo *msg) +ipcClientQuery::OnQueryComplete(nsresult status, const ipcmMessageClientInfo *msg) { NS_ASSERTION(mHandler, "no handler"); - PRUint32 nameCount = msg->NameCount(); - PRUint32 targetCount = msg->TargetCount(); - PRUint32 i; + PRUint32 nameCount = 0; + PRUint32 targetCount = 0; + const char **names = NULL; + const nsID **targets = NULL; - const char **names = (const char **) malloc(nameCount * sizeof(char *)); - const char *lastName = NULL; - for (i = 0; i < nameCount; ++i) { - lastName = msg->NextName(lastName); - names[i] = lastName; - } + if (NS_SUCCEEDED(status)) { + nameCount = msg->NameCount(); + targetCount = msg->TargetCount(); + PRUint32 i; - const nsID **targets = (const nsID **) malloc(targetCount * sizeof(nsID *)); - const nsID *lastTarget = NULL; - for (i = 0; i < targetCount; ++i) { - lastTarget = msg->NextTarget(lastTarget); - targets[i] = lastTarget; + names = (const char **) malloc(nameCount * sizeof(char *)); + const char *lastName = NULL; + for (i = 0; i < nameCount; ++i) { + lastName = msg->NextName(lastName); + names[i] = lastName; + } + + targets = (const nsID **) malloc(targetCount * sizeof(nsID *)); + const nsID *lastTarget = NULL; + for (i = 0; i < targetCount; ++i) { + lastTarget = msg->NextTarget(lastTarget); + targets[i] = lastTarget; + } } mHandler->OnQueryComplete(mQueryID, + status, mClientID, names, nameCount, targets, targetCount); + mHandler = NULL; - free(names); - free(targets); -} - -void -ipcClientQuery::OnQueryFailed(nsresult status) -{ - NS_ASSERTION(mHandler, "no handler"); - - mHandler->OnQueryFailed(mQueryID, status); - mHandler = nsnull; + if (names) + free(names); + if (targets) + free(targets); } //----------------------------------------------------------------------------- @@ -213,7 +214,7 @@ ipcService::OnIPCMClientInfo(const ipcmMessageClientInfo *msg) } if (!query->IsCanceled()) - query->OnQueryComplete(msg); + query->OnQueryComplete(NS_OK, msg); mQueryQ.DeleteFirst(); } @@ -230,7 +231,7 @@ ipcService::OnIPCMError(const ipcmMessageError *msg) } if (!query->IsCanceled()) - query->OnQueryFailed(NS_ERROR_FAILURE); + query->OnQueryComplete(NS_ERROR_FAILURE, NULL); mQueryQ.DeleteFirst(); } @@ -332,7 +333,7 @@ ipcService::CancelQuery(PRUint32 queryID) ipcClientQuery *query = mQueryQ.First(); while (query) { if (query->QueryID() == queryID) { - query->OnQueryFailed(NS_ERROR_ABORT); + query->OnQueryComplete(NS_BINDING_ABORTED, NULL); break; } query = query->mNext; @@ -432,7 +433,7 @@ ipcService::OnConnectionLost() // while (mQueryQ.First()) { ipcClientQuery *query = mQueryQ.First(); - query->OnQueryFailed(NS_ERROR_ABORT); + query->OnQueryComplete(NS_BINDING_ABORTED, NULL); mQueryQ.DeleteFirst(); } diff --git a/modules/ipc/test/TestIPC.cpp b/modules/ipc/test/TestIPC.cpp index f5d143b4087b..92e661c102d2 100644 --- a/modules/ipc/test/TestIPC.cpp +++ b/modules/ipc/test/TestIPC.cpp @@ -118,22 +118,17 @@ public: NS_IMPL_ISUPPORTS1(myIpcClientQueryHandler, ipcIClientQueryHandler) -NS_IMETHODIMP -myIpcClientQueryHandler::OnQueryFailed(PRUint32 aQueryID, nsresult aReason) -{ - printf("*** query failed [queryID=%u reason=0x%08x]\n", aQueryID, aReason); - return NS_OK; -} - NS_IMETHODIMP myIpcClientQueryHandler::OnQueryComplete(PRUint32 aQueryID, + nsresult aStatus, PRUint32 aClientID, const char **aNames, PRUint32 aNameCount, const nsID **aTargets, PRUint32 aTargetCount) { - printf("*** query complete [queryID=%u clientID=%u]\n", aQueryID, aClientID); + printf("*** query complete [queryID=%u status=0x%x clientID=%u]\n", + aQueryID, aStatus, aClientID); PRUint32 i; printf("*** names:\n");