bug #115
nead to clearify that all return values of "methods" are deep copies of values which must be freed by caller
| Status: | closed | Start date: | 09/04/2013 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assignee: | - | % Done: | 0% |
|
| Category: | - | |||
| Target version: | - |
Description
We have lots of C "methods" which expects an object and the return
value is a value of the object. Those return values are always copies.
Example:
drmaa2_jarray ja = drmaa2_jsession_get_job_array(js);
ja is newly allocated and must be freed by drmaa2_jarray_free(&ja).
Or:
drmaa2_string session_name = drmaa2_jarray_get_session_name(ja);
session_name is newly allocated and must be freed by drmaa2_string_free(&session_name).
This must be emphasized in the C spec somewhere in order to prevent DRMAA2 implementations which
just return a pointer to an internal values of the super object.
History
Updated by Peter Tröger about 9 years ago
- Status changed from submitted to accepted
With DRMAAv1, we had the situation that people understood the binding specification as end-user manual, which we really should avoid.
However, in this case, the spec may not be crystal clear even for the implementor. Section 2 has the following text:
" All structures have a specific support function for freeing them (drmaa2 * free). Implementations SHOULD perform the freeing of struct members automatically if the struct itself is freed."
This goes into the same direction, but is definitely not enough.
We could either extend Section 2, or add another Section with "user manual recommendations".
Updated by Stefan Klauck about 9 years ago
A general rule should be (current language binding and mock implementation):
Users have to call drmaa2_*_free for each object they retrieve by using drmaa2 library functions.
Objects are all returned variables, whose type starts with drmaa2_ and is not implemented as enum (i.e. drmaa2_error, drmaa2_bool and drmaa2_state (the other enums are currently not returned directly by library functions) ).
Updated by Daniel Gruber about 9 years ago
In DRMAA2 Grid Engine implementation we have following exceptions to the rule: * DRMAA2 dict accessors don't return deep copies (a dict is already a deep copy / main reason is because of performance). * The same for list accessors (traversing a list and always producing a copy and freeing is really not what we want). * Operation which are working on an array and returns one element of the given array. This is the wait for state function.
Updated by Redmine Admin about 8 years ago
I agree with the exceptions listed by Daniel -- doing deep copy in those places would not be useful, IMHO.
Updated by Daniel Gruber almost 8 years ago
As a result of OGF 42 in London I wrote a summary for the errata, when
copies and when pointer to existing objects needs to be returned:
When do we have to free for DRMAA2 functions?
DRMAA2 functions in C differ from the fact if return values are newly allocated or not. Unlike other APIs the DRMAA2 API functions can allocate the required memory for certain output values. The intention behind that is that it simplifies DRMAA2 applications. In case of errors such functions return either an error code or NULL if the newly allocated object should be returned.
Following table shows which functions are returning newly allocated objects which needs to be freed by the API caller in order to prevent memory leaks. As a rule of thumb a developer can remember that calls to previously allocated collections does return just a pointer to the collection while functions which return objects which do not exist in the memory accessible by the user are returning a copy of the element.
Following functions do return an object which needs to be freed:
drmaa2_get_drms_name
drmaa2_get_drms_version
drmaa2_describe_attribute
drmaa2_dict_create
drmaa2_dict_list
drmaa2_get_instance_value
drmaa2_get_jsession_names
drmaa2_get_rsession_names
drmaa2_jarray_get_id
drmaa2_jarray_get_jobs
drmaa2_jarray_get_job_template <—-name must be fixed other DRMAA2 issue
drmaa2_jarray_get_session_name
drmaa2_j_get_id
drmaa2_j_get_info
drmaa2_j_get_get_jt
drmaa2_j_get_session_name
drmaa2_jinfo_create
drmaa2_jinfo_impl_spec
drmaa2_jsession_get_contact
drmaa2_jsession_get_jarray
drmaa2_jsession_get_job_categories
drmaa2_jsession_get_jobs
drmaa2_jsession_get_session_name
drmaa2_jsession_run_bulk_jobs
drmaa2_jsession_run_job
drmaa2_jtemplate_create
drmaa2_jtemplate_impl_spec
drmaa2_lasterror_text
drmaa2_list_create
drmaa2_machineinfo_impl_spec
drmaa2_msession_get_all_jobs
drmaa2_msession_get_all_machines
drmaa2_msession_get_all_queues
drmaa2_msession_get_all_reservations
drmaa2_notification_impl_spec
drmaa2_open_jsession
drmaa2_open_msession
drmaa2_open_rsession
drmaa2_queueinfo_impl_spec
drmaa2_rinfo_impl_spec
drmaa2_rtemplate_impl_spec
Following functions do return an object which is a pointer to an existing object:
drmaa2_dict_get
drmaa2_list_get
drmaa2_jsession_wait_any_started
drmaa2_jsession_wait_any_terminated
Updated by Peter Tröger over 7 years ago
- Status changed from accepted to closed
Fixed in July 2015 errata.
(Other formats not available in this archive.