bug 526626: band-aids for shutdown assertions
--- a/dom/plugins/PluginModuleParent.cpp
+++ b/dom/plugins/PluginModuleParent.cpp
@@ -44,43 +44,50 @@
using mozilla::PluginLibrary;
using mozilla::ipc::NPRemoteIdentifier;
using namespace mozilla::plugins;
PR_STATIC_ASSERT(sizeof(NPIdentifier) == sizeof(void*));
+// static
PluginLibrary*
PluginModuleParent::LoadModule(const char* aFilePath)
{
_MOZ_LOG(__FUNCTION__);
// Block on the child process being launched and initialized.
PluginModuleParent* parent = new PluginModuleParent(aFilePath);
- parent->mSubprocess.Launch();
- parent->Open(parent->mSubprocess.GetChannel(),
- parent->mSubprocess.GetChildProcessHandle());
+ parent->mSubprocess->Launch();
+ parent->Open(parent->mSubprocess->GetChannel(),
+ parent->mSubprocess->GetChildProcessHandle());
return parent;
}
-PluginModuleParent::PluginModuleParent(const char* aFilePath) :
- mSubprocess(aFilePath)
+PluginModuleParent::PluginModuleParent(const char* aFilePath)
{
+ mSubprocess = new PluginProcessParent(aFilePath);
+ NS_ASSERTION(mSubprocess, "Out of memory!");
+
#ifdef DEBUG
PRBool ok =
#endif
mValidIdentifiers.Init();
NS_ASSERTION(ok, "Out of memory!");
}
PluginModuleParent::~PluginModuleParent()
{
+ if (mSubprocess) {
+ mSubprocess->Delete();
+ mSubprocess = nsnull;
+ }
}
PPluginInstanceParent*
PluginModuleParent::AllocPPluginInstance(const nsCString& aMimeType,
const uint16_t& aMode,
const nsTArray<nsCString>& aNames,
const nsTArray<nsCString>& aValues,
NPError* rv)
--- a/dom/plugins/PluginModuleParent.h
+++ b/dom/plugins/PluginModuleParent.h
@@ -195,17 +195,17 @@ private:
#if defined(XP_WIN) || defined(XP_MACOSX)
virtual nsresult NP_GetEntryPoints(NPPluginFuncs* pFuncs, NPError* error);
#endif
virtual nsresult NPP_New(NPMIMEType pluginType, NPP instance,
uint16_t mode, int16_t argc, char* argn[],
char* argv[], NPSavedData* saved,
NPError* error);
private:
- PluginProcessParent mSubprocess;
+ PluginProcessParent* mSubprocess;
const NPNetscapeFuncs* mNPNIface;
nsTHashtable<nsVoidPtrHashKey> mValidIdentifiers;
};
} // namespace plugins
} // namespace mozilla
#endif // ifndef dom_plugins_PluginModuleParent_h
--- a/dom/plugins/PluginProcessParent.cpp
+++ b/dom/plugins/PluginProcessParent.cpp
@@ -35,22 +35,28 @@
* the provisions above, a recipient may use your version of this file under
* the terms of any one of the MPL, the GPL or the LGPL.
*
* ***** END LICENSE BLOCK ***** */
#include "mozilla/plugins/PluginProcessParent.h"
#include "base/string_util.h"
+#include "mozilla/ipc/GeckoThread.h"
+using mozilla::ipc::BrowserProcessSubThread;
using mozilla::ipc::GeckoChildProcessHost;
+using mozilla::plugins::PluginProcessParent;
-namespace mozilla {
-namespace plugins {
-
+template<>
+struct RunnableMethodTraits<PluginProcessParent>
+{
+ static void RetainCallee(PluginProcessParent* obj) { }
+ static void ReleaseCallee(PluginProcessParent* obj) { }
+};
PluginProcessParent::PluginProcessParent(const std::string& aPluginFilePath) :
GeckoChildProcessHost(GeckoProcessType_Plugin),
mPluginFilePath(aPluginFilePath)
{
// XXXbent Need to catch crashing plugins by watching the process event!
}
@@ -61,11 +67,23 @@ PluginProcessParent::~PluginProcessParen
bool
PluginProcessParent::Launch()
{
std::vector<std::string> args;
args.push_back(mPluginFilePath);
return SyncLaunch(args);
}
+void
+PluginProcessParent::Delete()
+{
+ MessageLoop* currentLoop = MessageLoop::current();
+ MessageLoop* ioLoop =
+ BrowserProcessSubThread::GetMessageLoop(BrowserProcessSubThread::IO);
-} // namespace plugins
-} // namespace mozilla
+ if (currentLoop == ioLoop) {
+ delete this;
+ return;
+ }
+
+ ioLoop->PostTask(FROM_HERE,
+ NewRunnableMethod(this, &PluginProcessParent::Delete));
+}
--- a/dom/plugins/PluginProcessParent.h
+++ b/dom/plugins/PluginProcessParent.h
@@ -63,16 +63,18 @@ public:
PluginProcessParent(const std::string& aPluginFilePath);
~PluginProcessParent();
/**
* Synchronously launch the plugin process.
*/
bool Launch();
+ void Delete();
+
virtual bool CanShutdown()
{
return true;
}
using mozilla::ipc::GeckoChildProcessHost::GetShutDownEvent;
using mozilla::ipc::GeckoChildProcessHost::GetChannel;
using mozilla::ipc::GeckoChildProcessHost::GetChildProcessHandle;
--- a/ipc/glue/AsyncChannel.cpp
+++ b/ipc/glue/AsyncChannel.cpp
@@ -101,20 +101,31 @@ AsyncChannel::Open(Transport* aTransport
}
return true;
}
void
AsyncChannel::Close()
{
- mTransport->Close();
- // don't lose error-state information
- if (ChannelError != mChannelState)
- mChannelState = ChannelClosed;
+ // XXXcjones pretty much stuck with this bad parent/child
+ // dichotomy as long as the child's IO thread is the "main"
+ // thread and we don't have proper shutdown handling implemented.
+ if (mChild)
+ return OnClose();
+
+ AssertWorkerThread();
+
+ MutexAutoLock lock(mMutex);
+
+ mIOLoop->PostTask(
+ FROM_HERE, NewRunnableMethod(this, &AsyncChannel::OnClose));
+
+ while (ChannelConnected == mChannelState)
+ mCvar.Wait();
mTransport = NULL;
}
bool
AsyncChannel::Send(Message* msg)
{
AssertWorkerThread();
@@ -269,11 +280,28 @@ AsyncChannel::OnChannelOpened()
void
AsyncChannel::OnSend(Message* aMsg)
{
AssertIOThread();
mTransport->Send(aMsg);
// mTransport deletes aMsg
}
+void
+AsyncChannel::OnClose()
+{
+ AssertIOThread();
+
+ mTransport->Close();
+
+ // don't lose error-state information
+ if (ChannelError != mChannelState)
+ mChannelState = ChannelClosed;
+
+ if (!mChild) {
+ MutexAutoLock lock(mMutex);
+ mCvar.Notify();
+ }
+}
+
} // namespace ipc
} // namespace mozilla
--- a/ipc/glue/AsyncChannel.h
+++ b/ipc/glue/AsyncChannel.h
@@ -156,16 +156,17 @@ protected:
{
fprintf(stderr, "\n###!!! [%s][%s] Error: %s\n\n",
mChild ? "Child" : "Parent", channelName, msg);
}
// Run on the IO thread
void OnChannelOpened();
void OnSend(Message* aMsg);
+ void OnClose();
Transport* mTransport;
AsyncListener* mListener;
ChannelState mChannelState;
Mutex mMutex;
CondVar mCvar;
MessageLoop* mIOLoop; // thread where IO happens
MessageLoop* mWorkerLoop; // thread where work is done
--- a/ipc/glue/GeckoChildProcessHost.cpp
+++ b/ipc/glue/GeckoChildProcessHost.cpp
@@ -59,16 +59,22 @@ GeckoChildProcessHost::GeckoChildProcess
base::WaitableEventWatcher::Delegate* aDelegate)
: ChildProcessHost(RENDER_PROCESS), // FIXME/cjones: we should own this enum
mProcessType(aProcessType),
mMonitor("mozilla.ipc.GeckChildProcessHost.mMonitor"),
mLaunched(false),
mDelegate(aDelegate),
mChildProcessHandle(0)
{
+ MOZ_COUNT_CTOR(GeckoChildProcessHost);
+}
+
+GeckoChildProcessHost::~GeckoChildProcessHost()
+{
+ MOZ_COUNT_DTOR(GeckoChildProcessHost);
}
bool
GeckoChildProcessHost::SyncLaunch(std::vector<std::string> aExtraOpts)
{
MessageLoop* ioLoop =
BrowserProcessSubThread::GetMessageLoop(BrowserProcessSubThread::IO);
NS_ASSERTION(MessageLoop::current() != ioLoop, "sync launch from the IO thread NYI");
--- a/ipc/glue/GeckoChildProcessHost.h
+++ b/ipc/glue/GeckoChildProcessHost.h
@@ -56,16 +56,18 @@ protected:
typedef mozilla::Monitor Monitor;
public:
typedef base::ProcessHandle ProcessHandle;
GeckoChildProcessHost(GeckoProcessType aProcessType=GeckoProcessType_Default,
base::WaitableEventWatcher::Delegate* aDelegate=nsnull);
+ ~GeckoChildProcessHost();
+
bool SyncLaunch(std::vector<std::string> aExtraOpts=std::vector<std::string>());
bool AsyncLaunch(std::vector<std::string> aExtraOpts=std::vector<std::string>());
virtual void OnChannelConnected(int32 peer_pid);
virtual void OnMessageReceived(const IPC::Message& aMsg);
virtual void OnChannelError();
virtual bool CanShutdown() { return true; }
--- a/ipc/ipdl/test/cxx/IPDLUnitTestThreadChild.cpp
+++ b/ipc/ipdl/test/cxx/IPDLUnitTestThreadChild.cpp
@@ -60,13 +60,12 @@ IPDLUnitTestThreadChild::Init()
GeckoThread::Init();
IPDLUnitTestChildInit(channel(), GetParentProcessHandle(), owner_loop());
}
void
IPDLUnitTestThreadChild::CleanUp()
{
GeckoThread::CleanUp();
- IPDLUnitTestChildCleanUp();
}
} // namespace _ipdltest
} // namespace mozilla
--- a/ipc/ipdl/test/cxx/IPDLUnitTests.h
+++ b/ipc/ipdl/test/cxx/IPDLUnitTests.h
@@ -97,16 +97,14 @@ void IPDLUnitTestMain(void* aData);
//-----------------------------------------------------------------------------
// child process only
void IPDLUnitTestChildInit(IPC::Channel* transport,
base::ProcessHandle parent,
MessageLoop* worker);
-void IPDLUnitTestChildCleanUp();
-
} // namespace _ipdltest
} // namespace mozilla
#endif // ifndef mozilla__ipdltest_IPDLUnitTests_h
--- a/ipc/ipdl/test/cxx/IPDLUnitTests.template.cpp
+++ b/ipc/ipdl/test/cxx/IPDLUnitTests.template.cpp
@@ -183,27 +183,10 @@ IPDLUnitTestChildInit(IPC::Channel* tran
//-----------------------------------------------------------------------------
default:
fail("not reached");
return; // unreached
}
}
-void IPDLUnitTestChildCleanUp()
-{
- if (!gChildActor)
- return;
-
- switch (IPDLUnitTest()) {
-//-----------------------------------------------------------------------------
-//===== TEMPLATED =====
-${CHILD_CLEANUP_CASES}
-//-----------------------------------------------------------------------------
-
- default:
- fail("not reached");
- return; // unreached
- }
-}
-
} // namespace _ipdltest
} // namespace mozilla
--- a/ipc/ipdl/test/cxx/genIPDLUnitTests.py
+++ b/ipc/ipdl/test/cxx/genIPDLUnitTests.py
@@ -75,33 +75,21 @@ def main(argv):
%sChild** child =
reinterpret_cast<%sChild**>(&gChildActor);
*child = new %sChild();
(*child)->Open(transport, parent, worker);
return;
}
'''% (t, t, t, t) for t in unittests ])
- child_cleanup_cases = '\n'.join([
-''' case %s: {
- %sChild** child =
- reinterpret_cast<%sChild**>(&gChildActor);
- delete *child;
- *child = 0;
- return;
- }
-'''% (t, t, t) for t in unittests ])
-
-
templatefile = open(template, 'r')
sys.stdout.write(
string.Template(templatefile.read()).substitute(
INCLUDES=includes,
ENUM_VALUES=enum_values, LAST_ENUM=last_enum,
STRING_TO_ENUMS=string_to_enums,
ENUM_TO_STRINGS=enum_to_strings,
PARENT_MAIN_CASES=parent_main_cases,
- CHILD_INIT_CASES=child_init_cases,
- CHILD_CLEANUP_CASES=child_cleanup_cases))
+ CHILD_INIT_CASES=child_init_cases))
templatefile.close()
if __name__ == '__main__':
main(sys.argv)