Re: [PATCH 2/3] DSPBRIDGE: Move resource cleanup to bridge_release

[Ameya Palande <ameya.palande@nokia.com>]

Hi Ernesto,

ext Ramos Falcon, Ernesto wrote:
> Hi,
> 
> We have detected a use case where if an application creates a child process using fork call, and then the child and father processes call DSPProcessor_Attach() and create a new process context with new tgid; when the processes are terminated, only the last process calls bridge_release cleaning only the resources in the father process, leaving the child resources unreleased.
> 
> One solution we have seen is to perform goes through the entire process context list, clean up all the resources for all terminated processes or in "zombie" state, as below,
> 
> DRV_GetProcCtxtList(&pCtxtclosed, (struct DRV_OBJECT *)hDrvObject);
> while (pCtxtclosed != NULL) {
> 	printk("pCtxtclosed->pid = %d\n",pCtxtclosed->pid);
> 	tsk = find_task_by_pid(pCtxtclosed->pid);
> 
> 	if ((tsk == NULL) || (tsk->exit_state == EXIT_ZOMBIE)) {
> 	
> 		GT_1trace(driverTrace, GT_5CLASS,
> 			"***Task structure not existing for "
> 			 "process***%d\n", pCtxtclosed->pid);
> 		DRV_RemoveAllResources(pCtxtclosed);
> 		if (pCtxtclosed->hProcessor != NULL) {
> 					PROC_Detach
> 						 (pCtxtclosed->hProcessor);
> 		}
> 		pTmp = pCtxtclosed->next;
> 		DRV_RemoveProcContext((struct DRV_OBJECT *)hDrvObject,
> 					 pCtxtclosed,
> 					 (void *)pCtxtclosed->pid);
> 	} else {
> 		pTmp = pCtxtclosed->next;
> 	}
> 	pCtxtclosed = pTmp;
> }
> 
> Please let me know your comments.
> 
> /Ernesto

Good point :)

I would like to simplify this use case ;)

If we call DSPProcessor_Attach() twice in the same process and kill the process,
then it will leak memory for 1st instance of PROCESSOR object.

When we call open() on /dev/DspBridge a new PROCESS_CONTEXT is allocated, and it
should be allocated **only once** in bridge_open() unlike in NODE_Allocate() and
PROC_Attach(). PROCESS_CONTEXT tracks all the resources allocated on behalf of
an open file handle(and not the process / thread). When this handle is closed
all these resources should be freed in bridge_release(). Accountability of
resources should be done using PROCESS_CONTEXT and **not pid (which will be
different for different thread) / tgid (which will be different for parent and
child).

Above problem occurs because PROCESS_CONTEXT by design tracks only one PROCESSOR
object which gets freed in bridge_release().

Let me know your comments on this, and then we can proceed to fix this issue.

Cheers,
Ameya.