[Openais] Bug in main.c - libais_disconnect

Steven Dake sdake at redhat.com
Thu Apr 20 16:07:10 PDT 2006


Hans,

I have been in the process of rewriting this mechanism for a few days.

What I am trying to do is to create a thread per library connection.
This should improve performance and allow real-time apps to set their
scheduling policies so that the server side matches that of the client
side to avoid priority inversions and preemption pingpongs.  There is a
performance problem now with openais single-node which brought this work
about.

As an example, when a connection is opened to the executive, a message
is sent indicating the scheduling policy and priority.  The thread then
sets its scheduling policy and priority to match that of the client.
The priority of the aisexec main loop (running the timers and protocol)
is boosted from sched_other:0 to SCHED_RR:whatever is the highest
priority specified whenever a thread is created with higher priority.
This has the advantage of simplifying alot of the code.

My patch crashes now but it is coming close to working and I have
rewritten all of this code you have adjusted.  For now could you
workaround the problem as you work on AMF until I get the thread thing
reworked?

Regards
-steve


On Thu, 2006-04-20 at 11:05 +0200, Hans Feldt wrote:
> Sorry for spamming...
> 
> I freed private data on first call to disconnect, fixed in new patch.
> 
> Hans Feldt wrote:
> > Sorry, I removed too much code. New patch attached.
> > 
> > Hans Feldt wrote:
> > 
> >>
> >> I experience double freeing of memory in libais_disconnect when AMF is 
> >> restarting a component. Without understanding the bigger picture I 
> >> removed all partner stuff in the function except clearing the partner 
> >> pointer before freeing memory. The function will be called twice for 
> >> each of the two sockets service clients have. Therefore I thought each 
> >> call could free resources associated with the connection and nothing 
> >> else. Seems to work for me, patch attached.
> >>
> >> Regards,
> >> Hans
> >>
> >>
> >> ------------------------------------------------------------------------
> >>
> >> Index: main.c
> >> ===================================================================
> >> --- main.c    (revision 995)
> >> +++ main.c    (working copy)
> >> @@ -215,23 +215,17 @@
> >>      int res = 0;
> >>      struct outq_item *outq_item;
> >>  
> >> +    /*
> >> +     * Call library exit handler if any
> >> +     */
> >>      if (conn_info->should_exit_fn &&
> >>          ais_service[conn_info->service]->lib_exit_fn) {
> >>  
> >>          res = ais_service[conn_info->service]->lib_exit_fn (conn_info);
> >>      }
> >>  
> >> -    /*
> >> -     * Call library exit handler and free private data
> >> -     */
> >> -    if (conn_info->conn_info_partner &&
> >> -        conn_info->conn_info_partner->should_exit_fn &&
> >> -        
> >> ais_service[conn_info->conn_info_partner->service]->lib_exit_fn) {
> >> -
> >> -        res = 
> >> ais_service[conn_info->conn_info_partner->service]->lib_exit_fn 
> >> (conn_info->conn_info_partner);
> >> -        if (conn_info->private_data) {
> >> -            free (conn_info->private_data);
> >> -        }
> >> +    if (conn_info->private_data) {
> >> +        free (conn_info->private_data);
> >>      }
> >>  
> >>      /*
> >> @@ -257,43 +251,17 @@
> >>      }
> >>  
> >>      /*
> >> -     * Close the library connection and free its
> >> -     * data if it hasn't already been freed
> >> -     */
> >> -    if (conn_info->conn_info_partner &&
> >> -        conn_info->conn_info_partner->state != 
> >> CONN_STATE_DISCONNECTING) {
> >> -
> >> -        conn_info->conn_info_partner->state = CONN_STATE_DISCONNECTING;
> >> -
> >> -        close (conn_info->conn_info_partner->fd);
> >> -
> >> -        /*
> >> -         * Free the outq queued items
> >> -         */
> >> -        while (!queue_is_empty (&conn_info->conn_info_partner->outq)) {
> >> -            outq_item = queue_item_get 
> >> (&conn_info->conn_info_partner->outq);
> >> -            free (outq_item->msg);
> >> -            queue_item_remove (&conn_info->conn_info_partner->outq);
> >> -        }
> >> -
> >> -        queue_free (&conn_info->conn_info_partner->outq);
> >> -        if (conn_info->conn_info_partner->inb) {
> >> -            free (conn_info->conn_info_partner->inb);
> >> -        }
> >> -    }
> >> -
> >> -    /*
> >>       * If exit_fn didn't request a retry,
> >>       * free the conn_info structure
> >>       */
> >>      if (res != -1) {
> >> -        if (conn_info->conn_info_partner) {
> >> -            poll_dispatch_delete (aisexec_poll_handle,
> >> -                conn_info->conn_info_partner->fd);
> >> +        /*
> >> +         * update partners info about us
> >> +         */
> >> +        if (conn_info->conn_info_partner &&
> >> +            conn_info->conn_info_partner->conn_info_partner != NULL) {
> >> +            conn_info->conn_info_partner->conn_info_partner = NULL;
> >>          }
> >> -        poll_dispatch_delete (aisexec_poll_handle, conn_info->fd);
> >> -
> >> -        free (conn_info->conn_info_partner);
> >>          free (conn_info);
> >>      }
> >>  
> >>
> >>
> >> ------------------------------------------------------------------------
> >>
> >> _______________________________________________
> >> Openais mailing list
> >> Openais at lists.osdl.org
> >> https://lists.osdl.org/mailman/listinfo/openais
> > 
> > 
> > 
> > ------------------------------------------------------------------------
> > 
> > Index: main.c
> > ===================================================================
> > --- main.c	(revision 995)
> > +++ main.c	(working copy)
> > @@ -215,23 +215,17 @@
> >  	int res = 0;
> >  	struct outq_item *outq_item;
> >  
> > +	/*
> > +	 * Call library exit handler if any
> > +	 */
> >  	if (conn_info->should_exit_fn &&
> >  		ais_service[conn_info->service]->lib_exit_fn) {
> >  
> >  		res = ais_service[conn_info->service]->lib_exit_fn (conn_info);
> >  	}
> >  
> > -	/*
> > -	 * Call library exit handler and free private data
> > -	 */
> > -	if (conn_info->conn_info_partner &&
> > -		conn_info->conn_info_partner->should_exit_fn &&
> > -		ais_service[conn_info->conn_info_partner->service]->lib_exit_fn) {
> > -
> > -		res = ais_service[conn_info->conn_info_partner->service]->lib_exit_fn (conn_info->conn_info_partner);
> > -		if (conn_info->private_data) {
> > -			free (conn_info->private_data);
> > -		}
> > +	if (conn_info->private_data) {
> > +		free (conn_info->private_data);
> >  	}
> >  
> >  	/*
> > @@ -257,43 +251,19 @@
> >  	}
> >  
> >  	/*
> > -	 * Close the library connection and free its
> > -	 * data if it hasn't already been freed
> > -	 */
> > -	if (conn_info->conn_info_partner &&
> > -		conn_info->conn_info_partner->state != CONN_STATE_DISCONNECTING) {
> > -
> > -		conn_info->conn_info_partner->state = CONN_STATE_DISCONNECTING;
> > -
> > -		close (conn_info->conn_info_partner->fd);
> > -
> > -		/*
> > -		 * Free the outq queued items
> > -		 */
> > -		while (!queue_is_empty (&conn_info->conn_info_partner->outq)) {
> > -			outq_item = queue_item_get (&conn_info->conn_info_partner->outq);
> > -			free (outq_item->msg);
> > -			queue_item_remove (&conn_info->conn_info_partner->outq);
> > -		}
> > -
> > -		queue_free (&conn_info->conn_info_partner->outq);
> > -		if (conn_info->conn_info_partner->inb) {
> > -			free (conn_info->conn_info_partner->inb);
> > -		}
> > -	}
> > -
> > -	/*
> >  	 * If exit_fn didn't request a retry,
> >  	 * free the conn_info structure
> >  	 */
> >  	if (res != -1) {
> > -		if (conn_info->conn_info_partner) {
> > -			poll_dispatch_delete (aisexec_poll_handle,
> > -				conn_info->conn_info_partner->fd);
> > +		/*
> > +		 * update partners info about us
> > +		 */
> > +		if (conn_info->conn_info_partner &&
> > +			conn_info->conn_info_partner->conn_info_partner != NULL) {
> > +			conn_info->conn_info_partner->conn_info_partner = NULL;
> >  		}
> >  		poll_dispatch_delete (aisexec_poll_handle, conn_info->fd);
> >  
> > -		free (conn_info->conn_info_partner);
> >  		free (conn_info);
> >  	}
> >  
> > 
> > 
> > ------------------------------------------------------------------------
> > 
> > _______________________________________________
> > Openais mailing list
> > Openais at lists.osdl.org
> > https://lists.osdl.org/mailman/listinfo/openais
> 
> _______________________________________________
> Openais mailing list
> Openais at lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/openais




More information about the Openais mailing list