axis-c-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Mitchell (JIRA)" <j...@apache.org>
Subject [jira] Commented: (AXIS2C-783) Client seg faults if AXIS2C_HOME variable not set
Date Wed, 28 Nov 2007 03:59:43 GMT

    [ https://issues.apache.org/jira/browse/AXIS2C-783?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12546095
] 

Bill Mitchell commented on AXIS2C-783:
--------------------------------------

In doing some additional testing I uncovered additional error paths that generate a seg fault.
 These were revealed indirectly as a result of the introduction of the call to axis2_dep_engine_free
() above to avoid memory leaks.  To create these, I accidentally pointed the home directory
to a structure with valid xml files that enabled module addressing, but without the module
addressing DLL being present.  

This scenario uncovered three cleanup problems on error:

(1) In 3 places in dep_engine.c, it sets the error code AXIS2_ERROR_MODULE_VALIDATION_FAILED
    if (AXIS2_FAILURE == status)
    {
        axis2_repos_listener_free(dep_engine->repos_listener, env);
        axis2_conf_free(dep_engine->conf, env);
        AXIS2_ERROR_SET(env->error, AXIS2_ERROR_MODULE_VALIDATION_FAILED,
                        AXIS2_FAILURE);
        return NULL;
    }
In all 3 of these, after calling axis2_conf_free() the pointer to the free configuration should
be cleared:
        dep_engine->conf = NULL;
I observed 3 additional places axis2_dep_engine_load() in dep_engine.c where axis2_conf_free()
is invoked without clearing the pointer to the freed structure.  

(2) In axis2_dep_engine_do_deploy(), the path of the AXIS2_MODULE remembers the arch_reader
in dep_engine->arch_reader.  There are three places were the arch_reader is then freed
without clearing the dep_engine->arch_reader pointer.  These later result is a second free
of the same structure.  (See below for code changes.)

(3) Also in axis2_dep_engine_do_deploy(), the current file is remembered in dep_engine->curr_file.
 This pointer is not cleared upon error and should be.  Otherwise, as the file is also recorded
in the ws_to_deploy list, the file will be freed twice in axis2_dep_engine_free(), once as
the curr_file and again when the entire ws_to_deploy_list is freed.  

The following code of the trailing body of axis2_dep_engine_do_deploy() illustrates the changes
for (2) and (3):
            type = axis2_arch_file_data_get_type(dep_engine->curr_file, env);
            switch (type)
            {
                case AXIS2_SVC:
                    arch_reader = axis2_arch_reader_create(env);

                    svc_grp = axis2_svc_grp_create_with_conf(env, dep_engine->conf);
                    file_name = axis2_arch_file_data_get_name(dep_engine->
                                                              curr_file, env);
                    status = axis2_arch_reader_process_svc_grp(arch_reader, env,
                                                               file_name,
                                                               dep_engine, svc_grp);
                    if (AXIS2_SUCCESS != status)
                    {
                        axis2_arch_reader_free(arch_reader, env);
	   dep_engine->curr_file = NULL;
                        AXIS2_ERROR_SET(env->error, AXIS2_ERROR_INVALID_SVC,
                                        AXIS2_FAILURE);
                        return status;
                    }
                    status = axis2_dep_engine_add_new_svc(dep_engine, env, svc_grp);
                    if (AXIS2_SUCCESS != status)
                    {
                        axis2_arch_reader_free(arch_reader, env);
	   dep_engine->curr_file = NULL;
                        AXIS2_ERROR_SET(env->error, AXIS2_ERROR_INVALID_SVC,
                                        AXIS2_FAILURE);
                        return status;
                    }
                    dep_engine->curr_file = NULL;
                    break;
                case AXIS2_MODULE:
                    arch_reader = axis2_arch_reader_create(env);
                    if (dep_engine->arch_reader)
                    {
                        axis2_arch_reader_free(dep_engine->arch_reader, env);
                    }
                    dep_engine->arch_reader = axis2_arch_reader_create(env);
                    meta_data = axis2_module_desc_create(env);
                    file_name = axis2_arch_file_data_get_name(dep_engine->
                                                              curr_file, env);
                    status =
                        axis2_arch_reader_read_module_arch(env, file_name,
                                                           dep_engine, meta_data);
                    if (AXIS2_SUCCESS != status)
                    {
                        axis2_arch_reader_free(arch_reader, env);
	   dep_engine->arch_reader = NULL;
	   dep_engine->curr_file = NULL;
                        AXIS2_ERROR_SET(env->error, AXIS2_ERROR_INVALID_MODULE,
                                        AXIS2_FAILURE);
                        return AXIS2_FAILURE;
                    }
                    status = axis2_dep_engine_add_new_module(dep_engine, env,
                                                             meta_data);
                    if (AXIS2_SUCCESS != status)
                    {
                        axis2_arch_reader_free(arch_reader, env);
                        dep_engine->arch_reader = NULL;
	   dep_engine->curr_file = NULL;
                        AXIS2_ERROR_SET(env->error, AXIS2_ERROR_INVALID_MODULE,
                                        AXIS2_FAILURE);
                        return AXIS2_FAILURE;
                    }

                    dep_engine->curr_file = NULL;
                    break;
            }
            axis2_arch_reader_free(arch_reader, env);
            dep_engine->arch_reader = NULL;
            dep_engine->curr_file = NULL;
        }
    }
    return AXIS2_SUCCESS;

After introducing these fixes, when the axis2_mod_addr dll was not found in the addressing
directory, the client receives an Axis error 27, Module validation failed, and no seg fault.
 

> Client seg faults if AXIS2C_HOME variable not set
> -------------------------------------------------
>
>                 Key: AXIS2C-783
>                 URL: https://issues.apache.org/jira/browse/AXIS2C-783
>             Project: Axis2-C
>          Issue Type: Bug
>          Components: code generation, core/clientapi, core/deployment
>    Affects Versions: 1.1.0
>         Environment: Window XP, Visual Studio 2005
>            Reporter: Bill Mitchell
>            Assignee: Dinesh Premalal
>             Fix For: 1.2.0
>
>
> If the AXIS2C_HOME variable is not set, or if the client application code sets the ClientHome
argument to a directory that does not containt axis2.xml and related files, the client application
fails with a segmentation fault.  Investigating this uncovered several issues, best described
by highlighting the relevant actions in the code in chronological order:
> (1) axis2_dep_engine_check_client_home() in depengine.c detects the problem and stores
an error code, AXIS2_ERROR_CONFIG_NOT_FOUND (18).  But processing at this point is allowed
to continue. 
> (2) axis2_conf_builder_create_with_file_and_dep_engine_and_conf(), in particular axis2_desc_builder_create_with_file_and_dep_engine(),
then changes the status_code in the environment error  structure back to success.  This is
a side-effect of invoking the AXIS2_PARM_CHECK macro.  
> (3) In axis2_desc_builder_build_om(), the call to axiom_xml_reader_create_for_file()
returns an error, but the error number has now been replaced 
> with AXIS2_ERROR_CREATING_XML_STREAM_READER(141).  Axis2_desc_builder_build_om() then
replaces this error number with the same value, just to 
> make sure it is well and truly changed.  
> (4) In stub.c, axis2_stub_create_with_endpoint_ref_and_client_home() replaces the error
141 with a truly misleading AXIS2_ERROR_NO_MEMORY(2).  
> (5) In the generated stub code to create the service, axis2_stub_servicename_create()
ignores the fact that the returned stub pointer is zero and goes ahead and calls axis2_stub_servicename_populate_services()
anyway, where the code dies dereferencing the zero pointer.  
> Recommendations:
> (5) In the generated stub code for axis2_stub_servicename_create(), the fragment 
>     stub = axis2_stub_create_with_endpoint_ref_and_client_home ( env, endpoint_ref, client_home
);
>     axis2_stub_servicename_populate_services( stub, env );
>     return stub;
> should read:
>     stub = axis2_stub_create_with_endpoint_ref_and_client_home ( env, endpoint_ref, client_home
);
>     if (stub) 
>     {
>         axis2_stub_servicename_populate_services( stub, env );
>     }
>     return stub;
> (4) In stub.c, the procedure axis2_stub_create_with_endpoint_ref_and_client_home() really
needs to let the underlying error number be returned.  In the fragment:
>     /* create service_client*/
>     stub->svc_client = axis2_svc_client_create(env , client_home);
>     if (!stub->svc_client)
>     {
>         axis2_stub_free(stub, env);
>         AXIS2_ERROR_SET(env->error, AXIS2_ERROR_NO_MEMORY, AXIS2_FAILURE);
>         return NULL;
>     }
> the AXIS2_ERROR_SET invocation should be removed.  
> (1) Where the client has passed a clienthome parameter, but it does not point to a valid
Axis2C home directory structure, I prefer the first error code AXIS2_ERROR_CONFIG_NOT_FOUND
to the second AXIS2_ERROR_CREATING_XML_STREAM_READER.  So I suggest that axis2_dep_engine_load_client()
in depengine.c be changed to treat this as an immediate error instead of letting processing
continue.  The fragment:
>     if (client_home && 0 != axutil_strcmp("", client_home))
>     {
>         status = axis2_dep_engine_check_client_home(dep_engine, env, client_home);
>         if (AXIS2_SUCCESS == status)
>         {
>             is_repos_exist = AXIS2_TRUE;
>         }
>     }
>     else
> could be changed to:
>     if (client_home && 0 != axutil_strcmp("", client_home))
>     {
>         status = axis2_dep_engine_check_client_home(dep_engine, env, client_home);
>         if (AXIS2_SUCCESS != status)
>         {
>             return NULL;
>         }
>         is_repos_exist = AXIS2_TRUE;
>     }
>     else
> Of course, there may be a good reason to want to continue processing that I have not
uncovered.  In this case, you could instead do something to resolve item(2), such that the
original error code could be returned instead of overlaying it as happens at point (3).  

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: axis-c-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-c-dev-help@ws.apache.org


Mime
View raw message