[ 
https://issues.apache.org/jira/browse/PROTON-829?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14338757#comment-14338757
 ] 

Alan Conway commented on PROTON-829:
------------------------------------

Found the fix to the qpid problem as follows, I think there is still a (minor) 
issue for proton. 

The fix is to call pn_transport_free *after* pn_connection_free. The other way 
round works fine with proton 0.7 or 0.8 but causes memory errors and crashes 
proton 0.9.  
The API doc for transport_free says:
{code}
/**
 * Free a transport object.
 *
 * When a transport is freed, it is automatically unbound from its
 * associated connection.
 *
 * @param[in] transport a transport object or NULL
PN_EXTERN void pn_transport_free(pn_transport_t *transport);
*/
{code}
 
It does NOT say that the connection is freed. So either we need to modify the 
behavior so the connection is not freed (preferable since this is backwards 
compatible) or we need to change the API doc text to make it clear and release 
note the change.

> Possible reference counting bug in pn_clear_tpwork
> --------------------------------------------------
>
>                 Key: PROTON-829
>                 URL: https://issues.apache.org/jira/browse/PROTON-829
>             Project: Qpid Proton
>          Issue Type: Bug
>          Components: proton-c
>    Affects Versions: 0.8
>            Reporter: Alan Conway
>            Assignee: Alan Conway
>             Fix For: 0.9
>
>
> See QPID-6415 which describes a core dump in the qpid tests that appears when 
> using the current 0.9 proton master. The qpid tests pass OK with proton 0.8.
> The valgrind output in QPID-6415 shows that a connection is deleted while it 
> is being finalized by  a call from pn_connection_unbound to pn_clear_tpwork.
> I do not yet understand the details, but removing the following strange code 
> fixes the problem and passes the proton test suite without valgrind errors:
> {noformat}
> --- a/proton-c/src/engine/engine.c
> +++ b/proton-c/src/engine/engine.c
> @@ -690,10 +690,10 @@ void pn_clear_tpwork(pn_delivery_t *delivery)
>    {
>      LL_REMOVE(connection, tpwork, delivery);
>      delivery->tpwork = false;
> -    if (pn_refcount(delivery) > 0) {
> -      pn_incref(delivery);
> -      pn_decref(delivery);
> -    }
>    }
>  }
> {noformat}
> The code is strange because
> a) you should never examine a refcount except for debugging purposes
> b) under normal refcounting semantics incref+decref is a no-op.
> Is removing this code OK?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to