Solaris: fix a memory leak caused by calling ProcessList_getProcess twice for each LWP

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) {

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
 

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