search

kubernetes-client / c

Official C client library for Kubernetes
Apache License 2.0
141 stars 45 forks source link

Memory Leak #98

Closed minerba closed 2 years ago

minerba commented 2 years ago

Hi

I run valgrind with kubernetes c-client in my machine.

well ...

I found Memory Leak in all Api(v1_object_create() );

for example

 v1_node_local_var = v1_node_create (
        api_version ? strdup(api_version->valuestring) : NULL,
        kind ? strdup(kind->valuestring) : NULL,
        metadata ? metadata_local_nonprim : NULL,
        spec ? spec_local_nonprim : NULL,
        status ? status_local_nonprim : NULL
        );

    return v1_node_local_var;
end:
    return NULL;
}

v1_node_local_var = v1_node_create() but most parameters allocate dynamic memory in function. 

v1_node_t *v1_node_create(
    char *api_version,
    char *kind,
    v1_object_meta_t *metadata,
    v1_node_spec_t *spec,
    v1_node_status_t *status
    ) {
    v1_node_t *v1_node_local_var = malloc(sizeof(v1_node_t));
    if (!v1_node_local_var) {
        return NULL;
    }
    v1_node_local_var->api_version = api_version;
    v1_node_local_var->kind = kind;
    v1_node_local_var->metadata = metadata;
    v1_node_local_var->spec = spec;
    v1_node_local_var->status = status;

    return v1_node_local_var;
}

if(v1_node_local_var == NULL) , Failed to malloc in v1_node_create

NULL is returned without parameter Free.

Most api are operating in this way.

I think this could be a fatal problem.

ityuhui commented 2 years ago

malloc() failure is a rare case, even if it happens, program should exit because the system can no longer provide enough memory. Memory leak is not the most important thing at this time.

It isn't a "fatal" problem. And I suspect valgrind can not detect this issue.

Thank you for reporting this issue.

minerba commented 2 years ago

Hi @ityuhui I understand about your reply that It is not a "fatal" problem.

Well, this problem will be fixed?

ityuhui commented 2 years ago

I won't fix this issue recently because it's a rare case.

If you have interest, would you like to do some investigation and give a proposal ?

brendandburns commented 2 years ago

@minerba if you look at some of the other fixes that @ityuhui has done in the C generator (e.g. https://github.com/OpenAPITools/openapi-generator/pull/10756) it should be fairly straightforward to fix this issue (basically add free calls before returning NULL if malloc returns a NULL pointer.

I agree that for correctness, it is right to delete those pointers, but I also agree with @ityuhui that this is a rare case.

But if you fix the generator, we'd be happy to regenerate the code with the fix.

Thanks!

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten