rc: fix leaking runq->cmdfile
To reproduce run the following on a terminal: <snip> cpu% leak -s `{pstree | grep termrc | sed 1q | awk '{print $1}'} src(0x00209a82); // 12 src(0x0020b2a6); // 1 cpu% acid `{pstree | grep termrc | sed 1q | awk '{print $1}'} /proc/358/text:amd64 plan 9 executable /sys/lib/acid/port /sys/lib/acid/amd64 acid: src(0x0020b2a6) /sys/src/cmd/rc/plan9.c:169 164 if(runq->argv->words == 0) 165 poplist(); 166 else { 167 free(runq->cmdfile); 168 int f = open(runq->argv->words->word, 0); >169 runq->cmdfile = strdup(runq->argv->words->word); 170 runq->lexline = 1; 171 runq->pc--; 172 popword(); 173 if(f>=0) execcmds(openfd(f)); 174 } acid: </snap> Another `runq->cmdfile` leak is present here (captured on a cpu server): <snip> 277 ├listen [tcp * /rc/bin/service <nil>] 321 │├listen [/net/tcp/2 tcp!*!80] 322 │├listen [/net/tcp/3 tcp!*!17019] 324 ││└rc [/net/tcp/5 tcp!185.64.155.70!3516] 334 ││ ├rc -li 382 ││ │└pstree 336 ││ └rc 338 ││ └cat 323 │└listen [/net/tcp/4 tcp!*!17020] 278 ├listen [tcp * /rc/bin/service.auth <nil>] 320 │└listen [/net/tcp/1 tcp!*!567] 381 └closeproc cpu% leak -s 336 src(0x00209a82); // 2 src(0x002051d2); // 1 cpu% acid 336 /proc/336/text:amd64 plan 9 executable /sys/lib/acid/port /sys/lib/acid/amd64 acid: src(0x002051d2) /sys/src/cmd/rc/exec.c:1056 1051 1052 void 1053 Xsrcfile(void) 1054 { 1055 free(runq->cmdfile); >1056 runq->cmdfile = strdup(runq->code[runq->pc++].s); 1057 } acid: </snap> These leaks happen because we do not free cmdfile on all execution paths where `Xreturn()` is invoked. In `/sys/src/cmd/rc/exec.c:/^Xreturn` <snip> void Xreturn(void) { struct thread *p = runq; turfredir(); while(p->argv) poplist(); codefree(p->code); runq = p->ret; free(p); if(runq==0) Exit(getstatus()); } </snip> Note how the function `Xreturn()` frees a heap allocated instance of type `thread` with its members *except* the `cmdfile` member. On some code paths where `Xreturn()` is called there is an attempt to free `cmdfile`, however, there are some code paths where `Xreturn()` is called where `cmdfile` is not freed, leading to a leak. The attached patch calls `free(p->cmdfile)` in `Xreturn()` to avoid leaking memory and handling the free in one place. After applying the patch this particular leak is removed. There are still other leaks in rc: <snip> 277 ├listen [tcp * /rc/bin/service <nil>] 321 │├listen [/net/tcp/2 tcp!*!80] 322 │├listen [/net/tcp/3 tcp!*!17019] 324 ││└rc [/net/tcp/5 tcp!185.64.155.70!3516] 334 ││ ├rc -li 382 ││ │└pstree 336 ││ └rc 338 ││ └cat 323 │└listen [/net/tcp/4 tcp!*!17020] 278 ├listen [tcp * /rc/bin/service.auth <nil>] 320 │└listen [/net/tcp/1 tcp!*!567] 381 └closeproc cpu% leak -s 336 src(0x00209a82); // 2 src(0x002051d2); // 1 cpu% acid 336 /proc/336/text:amd64 plan 9 executable /sys/lib/acid/port /sys/lib/acid/amd64 acid: src(0x00209a82) /sys/src/cmd/rc/subr.c:9 4 #include "fns.h" 5 6 void * 7 emalloc(long n) 8 { >9 void *p = malloc(n); 10 if(p==0) 11 panic("Can't malloc %d bytes", n); 12 return p; 13 } 14 </snap> To help fixing those leaks emalloc(…) and erealloc(…) have been amended to use setmalloctag(…) and setrealloctag(…). The actual fixes for other reported leaks are *not* part of this merge and will follow.
This commit is contained in:
parent
a4c1f3cc18
commit
c7775b365e
2 changed files with 3 additions and 2 deletions
|
@ -465,6 +465,7 @@ Xreturn(void)
|
|||
turfredir();
|
||||
while(p->argv) poplist();
|
||||
codefree(p->code);
|
||||
free(p->cmdfile);
|
||||
runq = p->ret;
|
||||
free(p);
|
||||
if(runq==0)
|
||||
|
@ -937,8 +938,6 @@ Xrdcmds(void)
|
|||
Noerror();
|
||||
if(yyparse()){
|
||||
if(!p->iflag || p->eof && !Eintr()){
|
||||
if(p->cmdfile)
|
||||
free(p->cmdfile);
|
||||
closeio(p->cmdfd);
|
||||
Xreturn();
|
||||
}
|
||||
|
|
|
@ -9,6 +9,7 @@ emalloc(long n)
|
|||
void *p = malloc(n);
|
||||
if(p==0)
|
||||
panic("Can't malloc %d bytes", n);
|
||||
setmalloctag(p, getcallerpc(&n));
|
||||
return p;
|
||||
}
|
||||
|
||||
|
@ -18,6 +19,7 @@ erealloc(void *p, long n)
|
|||
p = realloc(p, n);
|
||||
if(p==0 && n!=0)
|
||||
panic("Can't realloc %d bytes\n", n);
|
||||
setrealloctag(p, getcallerpc(&p));
|
||||
return p;
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in a new issue