mirror of
				git://git.code.sf.net/p/cdesktopenv/code
				synced 2025-03-09 15:50:02 +00:00 
			
		
		
		
	Fix hash table memory leak when restoring PATH
There is a bug in path_alias() that may cause a memory leak when
clearing the hash table while setting/restoring PATH.
This applies a fix from Siteshwar Vashist:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01945.html
Note that, contrary to Siteshwar's analysis linked above, this bug
has nothing directly to do with subshells, forked or otherwise; it
can also be reproduced by temporarily setting PATH for a command,
for example, 'PATH=/dev/null true', and then doing a PATH search.
Modified analysis:
ksh maintains the value of PATH as a linked list. When a local
scope for PATH is created (e.g. in a virtual subshell or when doing
something like PATH=/foo/bar command ...), ksh duplicates PATH by
increasing the refcount for every element in the linked list by
calling the path_dup() and path_alias() functions. However, when
the state of PATH is restored, this refcount is not decreased. Next
time when PATH is reset to a new value, ksh calls the path_delete()
function to delete the linked list that stored the older path. But
the path_delete() function does not free elements whose refcount is
greater than 1, causing a memory leak.
src/cmd/ksh93/sh/path.c: path_alias():
- Decrease refcount and free old item if needed.
  (The 'old' variable was already introduced in 99065353, but
  its value was never used there; this fixes that as well.)
src/cmd/ksh93/tests/leaks.sh:
- Add regression test. With the bug, setting/restoring PATH
  (which clears the hash table) and doing a PATH search 16 times
  causes about 1.5 KiB of memory to be leaked.
			
			
This commit is contained in:
		
							parent
							
								
									5e7d335f2f
								
							
						
					
					
						commit
						361fe1fcc3
					
				
					 3 changed files with 21 additions and 1 deletions
				
			
		
							
								
								
									
										3
									
								
								NEWS
									
										
									
									
									
								
							
							
						
						
									
										3
									
								
								NEWS
									
										
									
									
									
								
							| 
						 | 
				
			
			@ -9,6 +9,9 @@ Any uppercase BUG_* names are modernish shell bug IDs.
 | 
			
		|||
 | 
			
		||||
- Fixed a crash when listing indexed arrays.
 | 
			
		||||
 | 
			
		||||
- Fixed a memory leak when restoring PATH when temporarily setting PATH
 | 
			
		||||
  for a command (e.g. PATH=/foo/bar command ...) or in a virtual subshell.
 | 
			
		||||
 | 
			
		||||
2020-07-07:
 | 
			
		||||
 | 
			
		||||
- Four of the date formats accepted by 'printf %()T' have had their
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -1800,7 +1800,9 @@ void path_alias(register Namval_t *np,register Pathcomp_t *pp)
 | 
			
		|||
		Pathcomp_t *old;
 | 
			
		||||
		nv_offattr(np,NV_NOPRINT);
 | 
			
		||||
		nv_stack(np,&talias_init);
 | 
			
		||||
		old = np->nvalue.pathcomp;
 | 
			
		||||
		old = (Pathcomp_t*)np->nvalue.cp;
 | 
			
		||||
		if (old && (--old->refcount <= 0))
 | 
			
		||||
			free((void*)old);
 | 
			
		||||
		np->nvalue.cp = (char*)pp;
 | 
			
		||||
		pp->refcount++;
 | 
			
		||||
		nv_setattr(np,NV_TAGGED|NV_NOFREE);
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -121,5 +121,20 @@ after=$(getmem)
 | 
			
		|||
(( after > before )) && err_exit 'unset of associative array causes memory leak' \
 | 
			
		||||
	"(leaked $((after - before)) $unit)"
 | 
			
		||||
 | 
			
		||||
# ======
 | 
			
		||||
# Memory leak when resetting PATH and clearing hash table
 | 
			
		||||
# ...steady memory state:
 | 
			
		||||
command -v ls >/dev/null	# add something to hash table
 | 
			
		||||
PATH=/dev/null true		# set/restore PATH & clear hash table
 | 
			
		||||
# ...test for leak:
 | 
			
		||||
before=$(getmem)
 | 
			
		||||
for	((i=0; i<16; i++))
 | 
			
		||||
do	PATH=/dev/null true	# set/restore PATH & clear hash table
 | 
			
		||||
	command -v ls		# do PATH search, add to hash table
 | 
			
		||||
done >/dev/null
 | 
			
		||||
after=$(getmem)
 | 
			
		||||
(( after > before )) && err_exit 'memory leak on PATH reset before subshell PATH search' \
 | 
			
		||||
	"(leaked $((after - before)) $unit)"
 | 
			
		||||
 | 
			
		||||
# ======
 | 
			
		||||
exit $((Errors<125?Errors:125))
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue