mirror of
				https://github.com/asterisk/asterisk.git
				synced 2025-10-31 18:55:19 +00:00 
			
		
		
		
	manager.c: Invalid ref-counting when purging events
We have a use-case where we generate a *lot* of events on the AMI, and
then when doing `manager show eventq` we would see some events which
would linger for hours or days in there. Obviously something was leaking.
Testing allowed us to track down this logic bug in the ref-counting on
the event purge.
Reproducing the bug was not super trivial, we managed to do it in a
production-like load testing environment with multiple AMI consumers.
The race condition itself:
1. something allocates and links `session`
2. `purge_sessions` iterates over that `session` (takes ref)
3. `purge_session` correctly de-referencess that session
4. `purge_session` re-evaluates the while() loop, taking a reference
5. `purge_session` exits (`n_max > 0` is false)
6. whatever allocated the `session` deallocates it, but a reference is
   now lost since we exited the `while` loop before de-referencing.
7. since the destructor is never called, the session->last_ev->usecount
   is never decremented, leading to events lingering in the queue
The impact of this bug does not seem major. The events are small and do
not seem, from our testing, to be causing meaningful additional CPU
usage. Mainly we wanted to fix this issue because we are internally
adding prometheus metrics to the eventq and those leaked events were
causing the metrics to show garbage data.
(cherry picked from commit 019d4ef17c)
			
			
This commit is contained in:
		
				
					committed by
					
						 George Joseph
						George Joseph
					
				
			
			
				
	
			
			
			
						parent
						
							2a6ba4937d
						
					
				
				
					commit
					ca086a587d
				
			| @@ -7466,7 +7466,9 @@ static int purge_sessions(int n_max) | ||||
| 	} | ||||
| 	i = ao2_iterator_init(sessions, 0); | ||||
| 	ao2_ref(sessions, -1); | ||||
| 	while ((session = ao2_iterator_next(&i)) && n_max > 0) { | ||||
|  | ||||
| 	/* The order of operations is significant */ | ||||
| 	while (n_max > 0 && (session = ao2_iterator_next(&i))) { | ||||
| 		ao2_lock(session); | ||||
| 		if (session->sessiontimeout && (now > session->sessiontimeout) && !session->inuse) { | ||||
| 			if (session->authenticated | ||||
|   | ||||
		Reference in New Issue
	
	Block a user