axis-c-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alastair FETTES" <afet...@mdacorporation.com>
Subject Axis2C patch and API comments
Date Thu, 03 Jan 2008 18:53:57 GMT
Apache Axis2/C Team:

During our work with Axis2/C version 1.1 we have run into a number of
issues that we would like to convey back to this group.  The main
problem that we have found is a lack of documentation regarding
ownership of memory in input parameters and return parameters.  Here is
the laundry list of items so far.

1.  Function axis2_svc_client_remove_all_headers:

<snippet>
98      for (i = 0; i < size; i++)
99      {
100         /*axiom_node_t *node = NULL;
101            node = axutil_array_list_get(svc_client->headers, env,i);
102
103            if (node)
104            {
105            axiom_node_free_tree(node, env);
106            node = NULL;
107            } */
108         axutil_array_list_remove(svc_client->headers, env, i);
109     }


110 AXIS2_EXTERN void *AXIS2_CALL
111 axutil_array_list_remove(
112     struct axutil_array_list *array_list,
113     const axutil_env_t * env,
114     int index)
115 {
116     void *result = NULL;
117     int i = 0;
118     AXIS2_PARAM_CHECK (env->error, array_list, AXIS2_FAILURE);
119
120     if (axutil_array_list_check_bound_exclusive(array_list, env,
index))
121     {
122         result = array_list->data[index];
123         for (i = index; i < array_list->size - 1; i++)
124             array_list->data[i] = array_list->data[i + 1];
125         array_list->size--;
126     }
127
128     return result;
129 }
</snippet>

Problem: There is a possible bug in
"axis2_svc_client_remove_all_headers" function.  The call to
"axutil_array_list_remove" (line 108) is where the problem stems from.
Since the for-loop (line 98) is incrementing, and the value of
"array_list->size" (line 125) is decrementing, eventually line 120 will
fail, and some of the headers will not be cleared out. 

Solution: The call to axutil_array_list_remove should be decrementing
(line 108 is a possible solution):

<snippet>
98      for (i = 0; i < size; i--)
99      {
100         /*axiom_node_t *node = NULL;
101            node = axutil_array_list_get(svc_client->headers, env,i);
102
103            if (node)
104            {
105            axiom_node_free_tree(node, env);
106            node = NULL;
107            } */
108         axutil_array_list_remove(svc_client->headers, env, (size -
i) - 1);
109     }
</snippet>

2.  Function axiom_element_create.

<snippet>
AXIS2_EXTERN
axiom_element_t* axiom_element_create(     
      const axutil_env_t*   env,
      axiom_node_t*         parent,
      const axis2_char_t*   localname,
      axiom_namespace_t*    ns,
      axiom_node_t**        node)
</snippet>

Problem: The axiom_node_t instance node has responsibility for the
returned axion_element_t*.  Therefore, calls to axiom_node_free_tree
will free this returned element (in addition to all the child nodes of
node).  The memory allocated for the return value should not be
de-allocated using axiom_element_free in this scenario, since it is
internally tied to the node.

Solution: Document memory ownership for parameters and return value.

3.  Function axiom_element_set_text:

<snippet>
AXIS2_EXTERN
axis2_status_t axiom_element_set_text(
      axiom_element_t*       om_element,
      const axutil_env_t*    env,
      const axis2_char_t*    text,
      axiom_node_t*          element_node)
</snippet>

Problem: Currently, the om_element has no use and is not used by the
function axiom_element_set_text therefore could probably be set NULL.
The function does not take control of the string text within text.  

Solution: Document memory ownership for parameters and return value.

4.  Function axiom_xml_reader_create_for_memory

<snippet>
AXIS2_EXTERN
axiom_xml_reader_t* axiom_xml_reader_create_for_memory(
      const axutil_env_t*     env,
      void*                   container,
      int                     size,
      const axis2_char_t*     encoding,
      int                     type)
</snippet>

Problem: The return value of type axiom_xml_reader_t* takes control of
the data contained within container.

Solution: Document memory ownership for parameters and return value.

5.  Function axiom_stax_builder_create

<snippet>
AXIS2_EXTERN
axiom_stax_builder_t* axiom_stax_builder_create(
      const axutil_env_t*   env,
      axiom_xml_reader_t*   parser)
</snippet>

Problem: The return value of type axiom_stax_t* takes responsibility for
the memory allocated for parser.

Solution: Document memory ownership for parameters and return value.

6.  Function axiom_document_create:

<snippet>
AXIS2_EXTERN
axiom_document_t* axiom_document_create(
        const axutil_env_t*     env,
        axiom_node_t*           root,
        axiom_stax_builder_t*   builder)
</snippet>

Problem: The return value of type axiom_document_t* takes responsibility
for the memory allocated for root. Any child node that is added to root
will be free-ed when the function axiom_document_free is called. The
return value of type axiom_document_t* does not takes responsibility for
the memory allocated for builder (as of v1.1.0, this seems odd, since
the internal structure maintains a pointer to builder, but does not free
builder. So this may create a dangling pointer situation). 

Solution: Document memory ownership for parameters and return value.
Investigate use of pointers in internal structure.

7.  Function axiom_document_build_all:

<snippet>
AXIS2_EXTERN
axiom_node_t* axiom_document_build_all(
      struct axiom_document*     document,
      const axutil_env_t*        env)
</snippet>

Problem: The caller of this function is not given responsibility for the
return value of type axiom_node_t*, since (as of v1.1.0) the return
value is free-ed by the function axiom_document_free. If the caller
calls the function axiom_document_free_self, the return value is NOT
free-ed. So a memory leak is possible here.

Solution: Document memory ownership for parameters and return value.
Investigate the use of free functions and pointers in regards to
possible memory leak.

8.  Function axiom_namespace_create:

<snippet>
AXIS2_EXTERN
axiom_namespace_t* axiom_namespace_create(
     const axutil_env_t*     env,
     const axis2_char_t*     uri,
     const axis2_char_t*     prefix)
</snippet>

Problem: This function and return value of type axiom_namespace_t* does
not take responsibility for the memory allocated for the parameters uri,
prefix, and of course env.

Solution: Document memory ownership for parameters and return value.

9.  Function axiom_attribute_create:

<snippet>
AXIS2_EXTERN
axiom_attribute_t* axiom_attribute_create(
     const axutil_env_t*     env,
     const axis2_char_t*     localname,
     const axis2_char_t*     value,
     axiom_namespace_t*      ns)
</snippet>

Problem: This return value of type axiom_attribute_t* does not take
responsibility for the memory allocated for localname, value, ns, and
env. Although, if the return value is added to an axiom_node_t using
axiom_element_add_attribute, the responsibility of the namespace of the
attribute is given to the axiom_node_t.  

Solution: Document memory ownership for parameters and return value.
Investigate the ownership of the namespace parameter.

10.  Function axiom_element_add_attribute:

<snippet>
AXIS2_EXTERN
axis2_status_t axiom_element_add_attribute(
     axiom_element_t*       om_element,
     const axutil_env_t*    env,
     axiom_attribute_t*     attribute,
     axiom_node_t*          element_node)
</snippet>

Problem: The parameter om_element takes responsibility for the memory
allocated for the parameter attribute, thus a call to axiom_element_free
will de-allocated the data for attribute. The parameter om_element also
takes responsibility for the namespace associated with attribute. 

Solution: Document memory ownership for parameters and return value.

11.  Function axutil_env_create_all:

<snippet>
AXIS2_EXTERN
axutil_env_t* axutil_env_create_all(
     const axis2_char_t*         log_file,
     const axutil_log_levels_t   log_level)
</snippet>

Problem: This function does not take responsibility for the memory
allocated by log_file. 

Solution: Document memory ownership for parameters and return value.

12.  Function axiom_output_create:

<snippet>
AXIS2_EXTERN
axiom_output_t* axiom_output_create(
     const axutil_env_t*     env,
     axiom_xml_writer_t*     xml_writer)
</snippet>

Problem: The return value of type axiom_output_t* takes responsibility
for the memory allocated to the parameter xml_writer. 

Solution: Document memory ownership for parameters and return value.

13.  Function axis2_svc_client_send_receive_non_blocking:

<snippet>
AXIS2_EXTERN
void axis2_svc_client_send_receive_non_blocking(
      axis2_svc_client_t*       svc_client,
      const axutil_env_t*     env,
      const axiom_node_t*     payload,
      axis2_callback_t*     callback) 
</snippet>

Problem: This function takes responsibility for the memory allocated for
the parameter payload (assumed due to access violations for free-ing),
as well as the custom headers that have been added to the parameter
svc_client (again, assumed because of access violations on free-ing).
This function also takes responsibility for the memory allocated at
callback, through the following call tree:

    axis2_svc_client_send_receive_non_blocking
    axis2_svc_client_send_receive_non_blocking_with_op_qname
    axis2_op_client_set_callback

The memory is free-ed with a call to axis2_op_client_free (it is assumed
that someone calls this)

Solution: Document memory ownership for parameters and return value.

14.  Function axis2_svc_client_add_header:

<snippet>
AXIS2_EXTERN
axis2_status_t axis2_svc_client_add_header(
      axis2_svc_client_t*   svc_client,
      const axutil_env_t*   env,
      axiom_node_t*         header)
</snippet>

The parameter svc_client does not take responsibility for the memory
allocated for header, although it does maintain an internal pointer to
the data. This can result in dangling pointer situations if the caller
deletes the memory passed using header before svc_client has a chance to
remove the header. 

Solution: Document memory ownership for parameters and return value.
Investigate the use of pointers in the internal structure to avoid
dangling pointer situations.

15.  Function axis2_svc_client_remove_all_headers:

<snippet>
AXIS2_EXTERN
axis2_status_t axis2_svc_client_remove_all_headers(
      axis2_svc_client_t*    svc_client,
      const axutil_env_t*    env)
</snippet>

This function does not take responsibility for the memory allocated to
the pointers passed to it. In addition, the memory passed to the
function axis2_svc_client_add_header through the parameter header is not
de-allocated by this function (axis2_svc_client_remove_all_headers).
This can create a possible memory leak. When in used in conjunction with
a call to axis2_svc_client_send_receive_non_blocking, this is not a
problem as this function will clean up the headers itself (this is
assumed, since when you free the header nodes, an access violations
occur when the SOAP message is sent and the calling function tries to
free the SOAP message data, i.e. the header nodes).

Solution: Document memory ownership for parameters and return value.
Investigate potential memory leak.

<end/>

Thanks very much for your time and efforts.

Alastair Fettes

This e-mail and any attachments are intended solely for the use of the
intended recipient(s) and may contain legally privileged, proprietary
and/or confidential information.  Any use, disclosure, dissemination,
distribution or copying of this e-mail and any attachments for any
purposes that have not been specifically authorized by the sender is
strictly prohibited.  If you are not the intended recipient, please
immediately notify the sender by reply e-mail and permanently delete all
copies and attachments.

The entire content of this e-mail is for "information purposes" only and
should not be relied upon by the recipient in any way unless otherwise
confirmed in writing by way of letter or facsimile. 


Mime
View raw message