Libcoap memory release, a missing functionality? A question.

[jonnyns]

For the record, I am currently using libcoap version 4.3.5. And, let me emphasize: this is currently a question only.

In the struct coap_session_t exists a variable app which is intended to hold application-specific data.

I can set and get data using the functions

void coap_session_set_app_data(coap_session_t *session, void *data);

void *coap_session_get_app_data(const coap_session_t *session);

All this works well.

Here comes my issue:

My application needs to store several data items with the session, for this I use an application-defined
struct which is dynamically allocated using malloc(). coap_session_set_app_data() is used to give this to the session.

The problem I experience is when the application is terminated, when one or many sessions are still active.
When the context is released, everything is cared for, except the release of memory which is held by the
session through use of coap_session_set_app_data(). This memory will never be freed.

From my point-of-view I miss a callback function which is called when the context is released, such that
all allocated memory can be correctly released, i.e. also memory allocated at application level.

I my own "fork" of libcoap I have introduced a

typedef void (*coap_session_release_cbf_t)(coap_session_t* session);

which is called when iterating through sessions at the time of context release. Similarly, I have

void coap_session_set_release_cbf(coap_session_release_cbf_t cbf);

to set this callback function pointer from the application. By this the application will always have a way to
release any application defined memory.

This measure seems to solve my issues just fine.

So the question is: it this not a functionality missing in the current public version of libcoap???
Or... am i missing something I am not currently seeing?

[mrdeep1]

Thanks for raising this.

There are the following storing of data functions which potentially have similar issues

coap_async_set_app_data()
coap_cache_set_app_data()
coap_context_set_app_data()
coap_session_set_app_data()

coap_cache_set_app_data() does have the mechanism for setting a call-back handler for releasing the app pointer when the cache object is deleted, but does not release the previous data if invoked for a second or subsequent time on the same cache object.

I see no reason as to why these functions can have a new variant that also sets an optional call-back handler for releasing the information pointed to by app pointer. I think that a subsequent call to the same set_app_data() function should release the previous data as per defined previous call-back handler.

That said, in your case, if you are only doing this server side, then you can trap in the event call-back handler the event COAP_EVENT_SERVER_SESSION_DEL and release the data. See #839.. It is not so easy to do this on the client side as COAP_EVENT_SESSION_CLOSED / COAP_EVENT_SESSION_FAILED occur earlier in the session shutdown process.

[jonnyns]

Thanks for your swift and detailed answer.
Actually, currently my change is for the client side. The (Raspberry Pi based) libcoap client handles some 20 light-weight servers carrying a variety of sensors and sensor technology.
The servers are actually Arduino Nano 33 IOT for minimum complexity (not using libcoap).

Anyway, I am rather new with libcoap, so I do not feel confident that my libcoap overview is sufficient to propose detailed modifications.
However, I can inform what I did modify in order to solve my issues.

coap_session.h: Added:

typedef void (*coap_session_release_cbf_t)(coap_session_t* session);

coap_session_release_cbf_t coap_session_release_cbf;

void coap_session_set_release_cbf(coap_session_release_cbf_t cbf);

coap_session.c: Added:

coap_session_release_cbf_t coap_session_release_cbf = NULL;

void coap_session_set_release_cbf(coap_session_release_cbf_t cbf) {
	coap_session_release_cbf = cbf;
}

coap_net.c: in coap_free_context_lkd():

changed lines from:

#if COAP_CLIENT_SUPPORT
  coap_session_t *sp, *rtmp;

  SESSIONS_ITER_SAFE(context->sessions, sp, rtmp) {
    coap_session_release_lkd(sp);
  }
#endif /* COAP_CLIENT_SUPPORT */

into:

#if COAP_CLIENT_SUPPORT
  coap_session_t *sp, *rtmp;

  SESSIONS_ITER_SAFE(context->sessions, sp, rtmp) {
		if (coap_session_release_cbf)         // Added
			(coap_session_release_cbf)(sp);	// Added
    coap_session_release_lkd(sp);
  }
#endif /* COAP_CLIENT_SUPPORT */

I leave any judgement of my changes to the libcoap author(s). There might be other/better/different ways of handling
this. Until I find the necessary modifications in a newer version of libcoap, I am happy with my current alterations, though.