From b8ded97e2da7e03a74bb73c3c279a501358cfe13 Mon Sep 17 00:00:00 2001 From: "edburns%acm.org" Date: Thu, 9 Nov 2006 20:10:17 +0000 Subject: [PATCH] Memory audit of code in src directory. Found and fixed some leaks. Declaring this directory memory clean. M src/Pluglet.cpp M src/Pluglet.h - use do_GetService instead of servman->GetServiceByContractID - do not keep a reference to plugletEngine as an ivar M src/PlugletFactory.cpp M src/PlugletFactory.h - use nsnull instead of NULL - do not keep a reference to plugletEngine as an ivar - use PL_strdup and PL_strfree to duplicate and free strings M src/PlugletsDir.cpp - use PL_strdup and PL_strfree to duplicate and free strings --- java/plugins/src/Pluglet.cpp | 25 +++++++--- java/plugins/src/Pluglet.h | 1 - java/plugins/src/PlugletFactory.cpp | 73 +++++++++++++++-------------- java/plugins/src/PlugletFactory.h | 1 - java/plugins/src/PlugletsDir.cpp | 17 +++++-- 5 files changed, 68 insertions(+), 49 deletions(-) diff --git a/java/plugins/src/Pluglet.cpp b/java/plugins/src/Pluglet.cpp index 5b2bde27a434..729d4f4d4e80 100644 --- a/java/plugins/src/Pluglet.cpp +++ b/java/plugins/src/Pluglet.cpp @@ -42,13 +42,10 @@ static NS_DEFINE_IID(kIPluginInstanceIID, NS_IPLUGININSTANCE_IID); NS_IMPL_ISUPPORTS1(Pluglet,nsIPluginInstance); -Pluglet::Pluglet(jobject object) : plugletEngine(nsnull), peer(nsnull) { - nsIServiceManager *servman = nsnull; - NS_GetServiceManager(&servman); +Pluglet::Pluglet(jobject object) : peer(nsnull) { nsresult rv; - rv = servman->GetServiceByContractID(PLUGLETENGINE_ContractID, - NS_GET_IID(iPlugletEngine), - getter_AddRefs(plugletEngine)); + nsCOMPtr plugletEngine = + do_GetService(PLUGLETENGINE_ContractID, &rv); if (NS_FAILED(rv)) { PR_LOG(PlugletLog::log, PR_LOG_DEBUG, ("Pluglet::Pluglet: Cannot access iPlugletEngine service\n")); @@ -74,7 +71,8 @@ Pluglet::~Pluglet() { Registry::Remove(jthis); JNIEnv *jniEnv = nsnull; nsresult rv; - plugletEngine = do_GetService(PLUGLETENGINE_ContractID, &rv); + nsCOMPtr plugletEngine = + do_GetService(PLUGLETENGINE_ContractID, &rv); if (plugletEngine) { rv = plugletEngine->GetJNIEnv(&jniEnv); if (NS_FAILED(rv)) { @@ -103,6 +101,9 @@ NS_METHOD Pluglet::Initialize(nsIPluginInstancePeer* _peer) { ("Pluglet::Initialize\n")); JNIEnv *env = nsnull; nsresult rv; + nsCOMPtr plugletEngine = + do_GetService(PLUGLETENGINE_ContractID, &rv); + rv = plugletEngine->GetJNIEnv(&env); if (NS_FAILED(rv)) { @@ -175,6 +176,8 @@ NS_METHOD Pluglet::Start(void) { ("Pluglet::Start\n")); JNIEnv *env = nsnull; nsresult rv; + nsCOMPtr plugletEngine = + do_GetService(PLUGLETENGINE_ContractID, &rv); rv = plugletEngine->GetJNIEnv(&env); if (NS_FAILED(rv)) { PR_LOG(PlugletLog::log, PR_LOG_DEBUG, @@ -193,6 +196,8 @@ NS_METHOD Pluglet::Stop(void) { ("Pluglet::Stop\n")); JNIEnv *env = nsnull; nsresult rv; + nsCOMPtr plugletEngine = + do_GetService(PLUGLETENGINE_ContractID, &rv); rv = plugletEngine->GetJNIEnv(&env); if (NS_FAILED(rv)) { PR_LOG(PlugletLog::log, PR_LOG_DEBUG, @@ -211,6 +216,8 @@ NS_METHOD Pluglet::Destroy(void) { ("Pluglet::Destroy\n")); JNIEnv *env = nsnull; nsresult rv; + nsCOMPtr plugletEngine = + do_GetService(PLUGLETENGINE_ContractID, &rv); rv = plugletEngine->GetJNIEnv(&env); if (NS_FAILED(rv)) { PR_LOG(PlugletLog::log, PR_LOG_DEBUG, @@ -234,6 +241,8 @@ NS_METHOD Pluglet::NewStream(nsIPluginStreamListener** listener) { } nsresult rv; JNIEnv *env; + nsCOMPtr plugletEngine = + do_GetService(PLUGLETENGINE_ContractID, &rv); rv = plugletEngine->GetJNIEnv(&env); if (NS_FAILED(rv)) { PR_LOG(PlugletLog::log, PR_LOG_DEBUG, @@ -265,6 +274,8 @@ NS_METHOD Pluglet::SetWindow(nsPluginWindow* window) { if (view->SetWindow(window) == PR_TRUE) { nsresult rv; JNIEnv *env; + nsCOMPtr plugletEngine = + do_GetService(PLUGLETENGINE_ContractID, &rv); rv = plugletEngine->GetJNIEnv(&env); if (NS_FAILED(rv)) { PR_LOG(PlugletLog::log, PR_LOG_DEBUG, diff --git a/java/plugins/src/Pluglet.h b/java/plugins/src/Pluglet.h index 7a9afb5f94da..bfc8f7336c05 100644 --- a/java/plugins/src/Pluglet.h +++ b/java/plugins/src/Pluglet.h @@ -54,6 +54,5 @@ class Pluglet : public nsIPluginInstance { static jmethodID getValueMID; nsCOMPtr peer; PlugletView *view; - nsCOMPtr plugletEngine; }; #endif /* __Pluglet_h__ */ diff --git a/java/plugins/src/PlugletFactory.cpp b/java/plugins/src/PlugletFactory.cpp index 867861a5fcc2..951c5074731d 100644 --- a/java/plugins/src/PlugletFactory.cpp +++ b/java/plugins/src/PlugletFactory.cpp @@ -29,9 +29,9 @@ #include "nsServiceManagerUtils.h" -jmethodID PlugletFactory::createPlugletMID = NULL; -jmethodID PlugletFactory::initializeMID = NULL; -jmethodID PlugletFactory::shutdownMID = NULL; +jmethodID PlugletFactory::createPlugletMID = nsnull; +jmethodID PlugletFactory::initializeMID = nsnull; +jmethodID PlugletFactory::shutdownMID = nsnull; nsresult PlugletFactory::CreatePluginInstance(const char* aPluginMIMEType, void **aResult) { @@ -44,6 +44,8 @@ nsresult PlugletFactory::CreatePluginInstance(const char* aPluginMIMEType, void JNIEnv *env = nsnull; nsresult rv = NS_ERROR_FAILURE; + nsCOMPtr plugletEngine = + do_GetService(PLUGLETENGINE_ContractID, &rv); rv = plugletEngine->GetJNIEnv(&env); if (NS_FAILED(rv)) { @@ -71,6 +73,8 @@ nsresult PlugletFactory::Initialize(void) { ("PlugletFactory::Initialize\n")); JNIEnv *env = nsnull; nsresult rv; + nsCOMPtr plugletEngine = + do_GetService(PLUGLETENGINE_ContractID, &rv); rv = plugletEngine->GetJNIEnv(&env); if (NS_FAILED(rv)) { @@ -130,6 +134,8 @@ nsresult PlugletFactory::Shutdown(void) { } JNIEnv *env = nsnull; nsresult rv; + nsCOMPtr plugletEngine = + do_GetService(PLUGLETENGINE_ContractID, &rv); rv = plugletEngine->GetJNIEnv(&env); if (NS_FAILED(rv)) { @@ -155,41 +161,36 @@ nsresult PlugletFactory::GetMIMEDescription(const char* *result) { return (*result) ? NS_OK : NS_ERROR_FAILURE; } -PlugletFactory::PlugletFactory(const char *mimeDescription, const char *path) : plugletEngine(nsnull) { - jthis = NULL; - this->path = new char[strlen(path)+1]; - strcpy(this->path,path); - this->mimeDescription = new char[strlen(mimeDescription)+1]; - strcpy(this->mimeDescription,mimeDescription); - - nsresult rv = NS_ERROR_FAILURE; - plugletEngine = do_GetService(PLUGLETENGINE_ContractID, &rv); - if (NS_FAILED(rv)) { - PR_LOG(PlugletLog::log, PR_LOG_DEBUG, - ("Pluglet::PlugletFactory: Cannot access iPlugletEngine service\n")); - return; - } +PlugletFactory::PlugletFactory(const char *inMimeDescription, const char *inPath) : jthis(nsnull), path(PL_strdup(inPath)), mimeDescription(PL_strdup(inMimeDescription)) +{ } PlugletFactory::~PlugletFactory(void) { - if (path != NULL) { - delete[] path; + + if (this->path) { + PL_strfree(this->path); + this->path = nsnull; } - if (mimeDescription != NULL) { - delete[] mimeDescription; + if (this->mimeDescription) { + PL_strfree(this->mimeDescription); + this->mimeDescription = nsnull; } JNIEnv *env = nsnull; nsresult rv; - rv = plugletEngine->GetJNIEnv(&env); - - if (NS_FAILED(rv)) { - PR_LOG(PlugletLog::log, PR_LOG_DEBUG, - ("PlugletFactory::~PlugletFactory: plugletEngine->GetJNIEnv failed\n")); - return; - } - - if (env != NULL) { - env->DeleteGlobalRef(jthis); + nsCOMPtr plugletEngine = + do_GetService(PLUGLETENGINE_ContractID, &rv); + if (NS_SUCCEEDED(rv)) { + rv = plugletEngine->GetJNIEnv(&env); + + if (NS_FAILED(rv)) { + PR_LOG(PlugletLog::log, PR_LOG_DEBUG, + ("PlugletFactory::~PlugletFactory: plugletEngine->GetJNIEnv failed\n")); + return; + } + + if (env != nsnull) { + env->DeleteGlobalRef(jthis); + } } } @@ -197,7 +198,7 @@ PlugletFactory * PlugletFactory::Load(const char * path) { PR_LOG(PlugletLog::log, PR_LOG_DEBUG, ("PlugletFactory::Load\n")); char * mime = PlugletLoader::GetMIMEDescription(path); - PlugletFactory * result = NULL; + PlugletFactory * result = nsnull; if (mime) { result = new PlugletFactory(mime,path); //delete[] mime; //nb we have a strange exception here @@ -217,16 +218,16 @@ int PlugletFactory::Compare(const char *mimeType) { if (!mimeType) { return 0; } - char * terminator = strchr(mimeDescription,':'); // terminator have to be not equal to NULL; + char * terminator = strchr(mimeDescription,':'); // terminator have to be not equal to nsnull; char *p1 = mimeDescription; char *p2 = strchr(p1,';'); - while ( p1 != NULL && p1 < terminator ) { - size_t n = sizeof(char) * ( ( (p2 == NULL || p2 > terminator) ? terminator : p2) - p1 ); + while ( p1 != nsnull && p1 < terminator ) { + size_t n = sizeof(char) * ( ( (p2 == nsnull || p2 > terminator) ? terminator : p2) - p1 ); if (PL_strncasecmp(p1,mimeType,n) == 0) { return 1; } p1 = p2 ; - if (p2 != NULL) { + if (p2 != nsnull) { p2 = strchr(++p2,';'); p1++; } diff --git a/java/plugins/src/PlugletFactory.h b/java/plugins/src/PlugletFactory.h index 542e5d6d0df8..ab51b7c1cdaa 100644 --- a/java/plugins/src/PlugletFactory.h +++ b/java/plugins/src/PlugletFactory.h @@ -43,7 +43,6 @@ class PlugletFactory { char *mimeDescription; char *path; PlugletFactory(const char *mimeDescription,const char * path); - nsCOMPtr plugletEngine; }; #endif /* __PlugletFactory_h__ */ diff --git a/java/plugins/src/PlugletsDir.cpp b/java/plugins/src/PlugletsDir.cpp index 266227fd6600..bcbd9dcc03db 100644 --- a/java/plugins/src/PlugletsDir.cpp +++ b/java/plugins/src/PlugletsDir.cpp @@ -28,13 +28,15 @@ static PRIntn DeleteEntryEnumerator(PLHashEntry *he, PRIntn i, void *arg) { PLHashTable *hash = (PLHashTable *) arg; - PL_HashTableRemove(hash, he->key); - void *key = (void *) he->key; - nsMemory::Free(key); + char *key = (char *) he->key; + + PL_strfree(key); PlugletFactory *toDelete = (PlugletFactory *) he->value; delete toDelete; + + PL_HashTableRemove(hash, he->key); return 0; } @@ -102,14 +104,21 @@ nsresult PlugletsDir::LoadPluglets() { mimeType = nsnull; rv = plugletFactory->GetMIMEDescription(&mimeType); if (NS_SUCCEEDED(rv)) { + const char *key = PL_strdup(mimeType); + printf("debug: edburns: key: %s address: %p\n", + key, key); PLHashEntry *entry = PL_HashTableAdd(mMimeTypeToPlugletFacoryHash, - (const void *) PL_strdup(mimeType), + (const void *) key, plugletFactory); rv = (nsnull != entry) ? NS_OK : NS_ERROR_FAILURE; } } } + + if (path) { + PL_strfree(path); + } } return rv; }