Thread: clean up memory when catching errors?

Hi there,

i wonder how you guys keep your code AND your memory clean at the same time. since i am working myself through a OpenCL tutorial i found that there is a lot of initialization to be done. each step might end up exiting the main function. what to do with all the stuff allocated before? i tried to add up all necessary cleaning commands within each catch of an error, but somehow that makes the code look a bit bloated.

as an example i'll list a part of my code:

Code:

// retreive OpenCL device CPU or GPU    
   if (clGetDeviceIDs(platform_id, CL_DEVICE_TYPE_GPU, 1, &device_id, NULL) != CL_SUCCESS) {
        printf("unable to get device_id\n");
        return 1;
    }


// create context
    // intenionally set context properties NULL, to keep sample code small
    context = clCreateContext(NULL, 1, &device_id, NULL, NULL, NULL);
    if (err != CL_SUCCESS) {
        printf("unable to create context. ERROR CODE: %i\n", err);
        return 1;
    }


// create command queue
    command_queue = clCreateCommandQueue(context, device_id, 0, &err);
    if (err != CL_SUCCESS) {
        printf("unable to create command queue\n");


        // we should start cleaning up here, since we created a context before..
        clReleaseContext(context);


        return 1;
    }


// create program object
    program = clCreateProgramWithSource(context, 1, &ProgramSource, NULL, &err);
    if (err != CL_SUCCESS) {
        printf("unable to create program\n");


        // cleaning up what we already have
        clReleaseCommandQueue(command_queue);
        clReleaseContext(context);


        return 1;
    }

as you can see, the further i get, the more clRelease... stuff will add up. so at some point it will look like this (which i find is very much):

Code:

// set kernel arguments    
   if (clSetKernelArg(kernel, 0, sizeof(cl_mem), &data_buffer_1) ||
        clSetKernelArg(kernel, 1, sizeof(cl_mem), &data_buffer_2) != CL_SUCCESS) {
        printf("unable to set kernel args\n");


        // cleaning up what we already have
        clReleaseMemObject(data_buffer_1);
        clReleaseMemObject(data_buffer_2);
        clReleaseKernel(kernel);
        clReleaseProgram(program);
        clReleaseCommandQueue(command_queue);
        clReleaseContext(context);


        return 1;
   }

while reading the tutorial i found, that the sample code just cleans up at the end of the main function if everything goes well, but i think that will leave some garbage at the memory in case some error occurs (right?).

thanks in advance for your contribution

kind regards
christian

In general when the program exits, all memory will be reclaimed by the OS, so it's typically not something you need to worry about. That said, it's not bad practice to do so, and if this were a function you were calling without exiting the program, then it'd definitely be a problem if it leaked memory.

C doesn't make this sort of thing easy the way a language like C++ does (with RAII). In C this is typically done with goto:

Code:

int ret = 0;

a = acquire();
if(a is error) {
  ret = -1;
  goto out1;
}

b = acquire();
if(b is error) {
  ret = -2;
  goto out2;
}

c = acquire();
if(c is error) {
  ret = -3;
  goto out3;
}

...

release(c);

out3:
release(b);

out2:
release(a);

out1:
return ret;

It's tedious and error prone, but that's kind of what you get with C. Of course you'd have to adapt the above to the function at hand, e.g. should it always release memory or is it returning allocated memory, etc.

Something like this might work.

Code:

ret = 1;  // error return code

a = getA(&err);
if (err) goto getA_failed;

b = getB(&err);
if (err) goto getB_failed;

c = getC(&err);
if (err) goto getC_failed;

//...

ret = 0;  // success


getC_failed:
    releaseC(c);
getB_failed:
    releaseB(b);
getA_failed:
    releaseA(a);

return ret;

benign felicitations,
neopagan pantheist

thanks for your comments

so in that case it seems not to be as 'ill-reputed' to use a goto as it sometimes is
i think i like that approach most, since it also looks clean and readable to me.

In general when the program exits, all memory will be reclaimed by the OS, so it's typically not something you need to worry about.

as far as i understood this should be the case on most 'common' os, but that is not for sure on every embedded system right?


thanks again to all of you

Originally Posted by indoet93

Terima kasih untuk artikel

We don't understand what you said. Please post responses, and comments in English.

Thank you.

Originally Posted by indoet93

Terima kasih untuk artikel

Kamu bodoh

Originally Posted by Elkvis

I miss the like button.

Why was the "Like" button removed?

Originally Posted by Prelude

> so in that case it seems not to be as 'ill-reputed' to use a goto as it sometimes is

And that's the problem with "XYZ is evil!" arguments, it closes minds to a feature when it's reasonable or even the best option. I recall getting blasted here for using goto for flow control once, even though it was objectively the cleanest and most efficient approach.

> as far as i understood this should be the case on most 'common' os, but that is not for sure on every embedded system right?

Correct. That said, there are two leak situations that are different but ultimately have the same best practice:

• Leaking memory in a long running program.
• Leaking memory if the OS doesn't reclaim it on termination.

In both cases you clean up after yourself by releasing memory when it's no longer needed. The first case is your primary concern because it's more commonly an issue, and the second case is more for maximizing portability.

That said, one thing to note is if you're on a heavily restricted embedded environment, use of dynamic memory at all might be prohibitively expensive. The rules tend to change when you go embedded.

I agree about the "XYZ is evil!" dogma. Absolutely unproductive and can make worse software. It's unfortunate that it's so prevalent across the entire industry too. No matter the language, the paradigm, the company, someone will not doing something "just because".