From da4877f48c70f765f8bfb60c7668e8499055662e Mon Sep 17 00:00:00 2001 From: "Guy M. Broome" Date: Fri, 30 Mar 2018 14:34:12 -0400 Subject: [PATCH] Solaris: fix a memory leak caused by calling ProcessList_getProcess twice for each LWP --- solaris/SolarisProcess.c | 6 +- solaris/SolarisProcessList.c | 140 +++++++++++++++-------------------- solaris/SolarisProcessList.h | 2 +- 3 files changed, 65 insertions(+), 83 deletions(-) diff --git a/solaris/SolarisProcess.c b/solaris/SolarisProcess.c index 365e2ac1..0e766ba2 100644 --- a/solaris/SolarisProcess.c +++ b/solaris/SolarisProcess.c @@ -131,10 +131,10 @@ SolarisProcess* SolarisProcess_new(Settings* settings) { } void Process_delete(Object* cast) { - SolarisProcess* this = (SolarisProcess*) cast; + SolarisProcess* sp = (SolarisProcess*) cast; Process_done((Process*)cast); - free(this->zname); - free(this); + free(sp->zname); + free(sp); } void SolarisProcess_writeField(Process* this, RichString* str, ProcessField field) { diff --git a/solaris/SolarisProcessList.c b/solaris/SolarisProcessList.c index cd6c3528..2c681852 100644 --- a/solaris/SolarisProcessList.c +++ b/solaris/SolarisProcessList.c @@ -178,6 +178,7 @@ static inline void SolarisProcessList_scanMemoryInfo(ProcessList* pl) { uint64_t totalfree = 0; int nswap = 0; char *spath = NULL; + char *spathbase = NULL; // Part 1 - physical memory if (spl->kd != NULL) { meminfo = kstat_lookup(spl->kd,"unix",0,"system_pages"); } @@ -194,7 +195,7 @@ static inline void SolarisProcessList_scanMemoryInfo(ProcessList* pl) { // Not really "buffers" but the best Solaris analogue that I can find to // "memory in use but not by programs or the kernel itself" pl->buffersMem = (totalmem_pgs->value.ui64 - pages->value.ui64) * PAGE_SIZE_KB; - } else { + } else { // Fall back to basic sysconf if kstat isn't working pl->totalMem = sysconf(_SC_PHYS_PAGES) * PAGE_SIZE; pl->buffersMem = 0; @@ -204,9 +205,10 @@ static inline void SolarisProcessList_scanMemoryInfo(ProcessList* pl) { // Part 2 - swap nswap = swapctl(SC_GETNSWP, NULL); - if (nswap > 0) { sl = malloc(nswap * sizeof(swapent_t) + sizeof(int)); } - if (sl != NULL) { spath = malloc( nswap * MAXPATHLEN ); } - if (spath != NULL) { + if (nswap > 0) { sl = xMalloc((nswap * sizeof(swapent_t)) + sizeof(int)); } + if (sl != NULL) { spathbase = xMalloc( nswap * MAXPATHLEN ); } + if (spathbase != NULL) { + spath = spathbase; swapdev = sl->swt_ent; for (int i = 0; i < nswap; i++, swapdev++) { swapdev->ste_path = spath; @@ -220,20 +222,20 @@ static inline void SolarisProcessList_scanMemoryInfo(ProcessList* pl) { for (int i = 0; i < nswap; i++, swapdev++) { totalswap += swapdev->ste_pages; totalfree += swapdev->ste_free; - free(swapdev->ste_path); } - free(sl); } + free(spathbase); + free(sl); pl->totalSwap = totalswap * PAGE_SIZE_KB; pl->usedSwap = pl->totalSwap - (totalfree * PAGE_SIZE_KB); } -void ProcessList_delete(ProcessList* this) { - const SolarisProcessList* spl = (SolarisProcessList*) this; - if (spl->kd) kstat_close(spl->kd); +void ProcessList_delete(ProcessList* pl) { + SolarisProcessList* spl = (SolarisProcessList*) pl; + ProcessList_done(pl); free(spl->cpus); - ProcessList_done(this); - free(this); + if (spl->kd) kstat_close(spl->kd); + free(spl); } /* NOTE: the following is a callback function of type proc_walk_f @@ -245,76 +247,56 @@ void ProcessList_delete(ProcessList* this) { int SolarisProcessList_walkproc(psinfo_t *_psinfo, lwpsinfo_t *_lwpsinfo, void *listptr) { struct timeval tv; struct tm date; - bool preExistingP = false; - bool preExistingL = false; bool preExisting; - Process *cproc; - SolarisProcess *csproc; + pid_t getpid; // Setup process list ProcessList *pl = (ProcessList*) listptr; SolarisProcessList *spl = (SolarisProcessList*) listptr; - // Setup Process entry - Process *proc = ProcessList_getProcess(pl, _psinfo->pr_pid * 1024, &preExistingP, (Process_New) SolarisProcess_new); - SolarisProcess *sproc = (SolarisProcess*) proc; - - // Setup LWP entry id_t lwpid_real = _lwpsinfo->pr_lwpid; if (lwpid_real > 1023) return 0; - pid_t lwpid = proc->pid + lwpid_real; - Process *lwp = ProcessList_getProcess(pl, lwpid, &preExistingL, (Process_New) SolarisProcess_new); - SolarisProcess *slwp = (SolarisProcess*) lwp; - + pid_t lwpid = (_psinfo->pr_pid * 1024) + lwpid_real; bool onMasterLWP = (_lwpsinfo->pr_lwpid == _psinfo->pr_lwp.pr_lwpid); - - // Determine whether we're updating proc info or LWP info - // based on whether or not we're on the representative LWP - // for a given proc if (onMasterLWP) { - cproc = proc; - csproc = sproc; - preExisting = preExistingP; + getpid = _psinfo->pr_pid * 1024; } else { - cproc = lwp; - csproc = slwp; - preExisting = preExistingL; - } + getpid = lwpid; + } + Process *proc = ProcessList_getProcess(pl, getpid, &preExisting, (Process_New) SolarisProcess_new); + SolarisProcess *sproc = (SolarisProcess*) proc; gettimeofday(&tv, NULL); // Common code pass 1 - cproc->show = false; - csproc->zoneid = _psinfo->pr_zoneid; - csproc->zname = SolarisProcessList_readZoneName(spl->kd,sproc); - csproc->taskid = _psinfo->pr_taskid; - csproc->projid = _psinfo->pr_projid; - csproc->poolid = _psinfo->pr_poolid; - csproc->contid = _psinfo->pr_contract; - cproc->priority = _lwpsinfo->pr_pri; - cproc->nice = _lwpsinfo->pr_nice; - cproc->processor = _lwpsinfo->pr_onpro; - cproc->state = _lwpsinfo->pr_sname; - // This could potentially get bungled if the master LWP is not the first - // one enumerated. Unaware of any workaround right now. - if ((cproc->state == 'O') && !onMasterLWP) proc->state = 'O'; + proc->show = false; + sproc->taskid = _psinfo->pr_taskid; + sproc->projid = _psinfo->pr_projid; + sproc->poolid = _psinfo->pr_poolid; + sproc->contid = _psinfo->pr_contract; + proc->priority = _lwpsinfo->pr_pri; + proc->nice = _lwpsinfo->pr_nice; + proc->processor = _lwpsinfo->pr_onpro; + proc->state = _lwpsinfo->pr_sname; // NOTE: This 'percentage' is a 16-bit BINARY FRACTIONS where 1.0 = 0x8000 // Source: https://docs.oracle.com/cd/E19253-01/816-5174/proc-4/index.html // (accessed on 18 November 2017) - cproc->percent_mem = ((uint16_t)_psinfo->pr_pctmem/(double)32768)*(double)100.0; - cproc->st_uid = _psinfo->pr_euid; - cproc->user = UsersTable_getRef(pl->usersTable, proc->st_uid); - cproc->pgrp = _psinfo->pr_pgid; - cproc->nlwp = _psinfo->pr_nlwp; - cproc->comm = xStrdup(_psinfo->pr_fname); - cproc->commLen = strnlen(_psinfo->pr_fname,PRFNSZ); - cproc->tty_nr = _psinfo->pr_ttydev; - cproc->m_resident = _psinfo->pr_rssize/PAGE_SIZE_KB; - cproc->m_size = _psinfo->pr_size/PAGE_SIZE_KB; + proc->percent_mem = ((uint16_t)_psinfo->pr_pctmem/(double)32768)*(double)100.0; + proc->st_uid = _psinfo->pr_euid; + proc->pgrp = _psinfo->pr_pgid; + proc->nlwp = _psinfo->pr_nlwp; + proc->tty_nr = _psinfo->pr_ttydev; + proc->m_resident = _psinfo->pr_rssize/PAGE_SIZE_KB; + proc->m_size = _psinfo->pr_size/PAGE_SIZE_KB; if (!preExisting) { - csproc->realpid = _psinfo->pr_pid; - csproc->lwpid = lwpid_real; + sproc->realpid = _psinfo->pr_pid; + sproc->lwpid = lwpid_real; + sproc->zoneid = _psinfo->pr_zoneid; + sproc->zname = SolarisProcessList_readZoneName(spl->kd,sproc); + proc->user = UsersTable_getRef(pl->usersTable, proc->st_uid); + proc->comm = xStrdup(_psinfo->pr_fname); + proc->commLen = strnlen(_psinfo->pr_fname,PRFNSZ); } // End common code pass 1 @@ -326,7 +308,7 @@ int SolarisProcessList_walkproc(psinfo_t *_psinfo, lwpsinfo_t *_lwpsinfo, void * // See note above (in common section) about this BINARY FRACTION proc->percent_cpu = ((uint16_t)_psinfo->pr_pctcpu/(double)32768)*(double)100.0; proc->time = _psinfo->pr_time.tv_sec; - if(!preExistingP) { // Tasks done only for NEW processes + if(!preExisting) { // Tasks done only for NEW processes sproc->is_lwp = false; proc->starttime_ctime = _psinfo->pr_start.tv_sec; } @@ -347,35 +329,35 @@ int SolarisProcessList_walkproc(psinfo_t *_psinfo, lwpsinfo_t *_lwpsinfo, void * } proc->show = !(pl->settings->hideKernelThreads && sproc->kernel); } else { // We are not in the master LWP, so jump to the LWP handling code - lwp->percent_cpu = ((uint16_t)_lwpsinfo->pr_pctcpu/(double)32768)*(double)100.0; - lwp->time = _lwpsinfo->pr_time.tv_sec; - if (!preExistingL) { // Tasks done only for NEW LWPs - slwp->is_lwp = true; - lwp->basenameOffset = -1; - lwp->ppid = proc->pid; - lwp->tgid = proc->pid; - slwp->realppid = sproc->realpid; - lwp->starttime_ctime = _lwpsinfo->pr_start.tv_sec; + proc->percent_cpu = ((uint16_t)_lwpsinfo->pr_pctcpu/(double)32768)*(double)100.0; + proc->time = _lwpsinfo->pr_time.tv_sec; + if (!preExisting) { // Tasks done only for NEW LWPs + sproc->is_lwp = true; + proc->basenameOffset = -1; + proc->ppid = _psinfo->pr_pid * 1024; + proc->tgid = _psinfo->pr_pid * 1024; + sproc->realppid = _psinfo->pr_pid; + proc->starttime_ctime = _lwpsinfo->pr_start.tv_sec; } // Top-level process only gets this for the representative LWP - if (slwp->kernel && !pl->settings->hideKernelThreads) lwp->show = true; - if (!slwp->kernel && !pl->settings->hideUserlandThreads) lwp->show = true; + if (sproc->kernel && !pl->settings->hideKernelThreads) proc->show = true; + if (!sproc->kernel && !pl->settings->hideUserlandThreads) proc->show = true; } // Top-level LWP or subordinate LWP // Common code pass 2 if (!preExisting) { if ((sproc->realppid <= 0) && !(sproc->realpid <= 1)) { - csproc->kernel = true; + sproc->kernel = true; } else { - csproc->kernel = false; + sproc->kernel = false; } - (void) localtime_r((time_t*) &cproc->starttime_ctime, &date); - strftime(cproc->starttime_show, 7, ((cproc->starttime_ctime > tv.tv_sec - 86400) ? "%R " : "%b%d "), &date); - ProcessList_add(pl, cproc); + (void) localtime_r((time_t*) &proc->starttime_ctime, &date); + strftime(proc->starttime_show, 7, ((proc->starttime_ctime > tv.tv_sec - 86400) ? "%R " : "%b%d "), &date); + ProcessList_add(pl, proc); } - cproc->updated = true; + proc->updated = true; // End common code pass 2 diff --git a/solaris/SolarisProcessList.h b/solaris/SolarisProcessList.h index 8bebd897..a5f2fbc2 100644 --- a/solaris/SolarisProcessList.h +++ b/solaris/SolarisProcessList.h @@ -48,7 +48,7 @@ char* SolarisProcessList_readZoneName(kstat_ctl_t* kd, SolarisProcess* sproc); ProcessList* ProcessList_new(UsersTable* usersTable, Hashtable* pidWhiteList, uid_t userId); -void ProcessList_delete(ProcessList* this); +void ProcessList_delete(ProcessList* pl); /* NOTE: the following is a callback function of type proc_walk_f * and MUST conform to the appropriate definition in order