Software Mansion•9mo ago

unifex seg fault on handle_destroy_state

Hi, i'm implementing g772.1 decoder/encoder plugin and have issue with handle_destroy state. I've taken freeswitch g7221 implementation(https://github.com/traviscross/freeswitch/tree/master/libs/libg722_1/src). I have the following state:

#pragma once
#include <g722_1.h>

typedef struct State State;

struct State {
  g722_1_decode_state_t *decode_state;
};

#include "_generated/decoder.h"

#pragma once
#include <g722_1.h>

typedef struct State State;

struct State {
  g722_1_decode_state_t *decode_state;
};

#include "_generated/decoder.h"

and added following handle_destroy_state callback:

void handle_destroy_state(UnifexEnv *env, State *state) {
  UNIFEX_UNUSED(env);

  if (state->decode_state != NULL) {
    g722_1_decode_release(state->decode_state);
  }
}

void handle_destroy_state(UnifexEnv *env, State *state) {
  UNIFEX_UNUSED(env);

  if (state->decode_state != NULL) {
    g722_1_decode_release(state->decode_state);
  }
}

but it causes segmentation fault. g7221_1_decode_release calls free on passed pointer. Implementation of g722_1_decode_release:

int g722_1_decode_release(g722_1_decode_state_t *s)
{
    free(s);
    return 0;
}

int g722_1_decode_release(g722_1_decode_state_t *s)
{
    free(s);
    return 0;
}

Is it state freed by unifex itself?

4 Replies

spscreamOP•9mo ago

I hope this is right place to post unifex questions, since Membrane framework is the owner of unifex.

Qizot•9mo ago

Are you sure that you have initialized the state properly (assinging a NULL on initialization or making sure that the state gets initialized before being freed)? Operating with state should have the following steps: 1. unifex_alloc_state to allocate a state 2. unifex_release_state in the same initialization function just before returning from the init function to ensure the state gets released when the last elixir's reference vanishes 3. do any internal cleanup inside of handle_destroy_state, you don't have to release the unifex state inside of the callback, just cleanup after the g722

spscreamOP•9mo ago

My bad. I made mistake on state initialization, made changes to create:

UNIFEX_TERM create(UnifexEnv *env, int bit_rate, int sample_rate) {
  UNIFEX_TERM res;

  State *state = unifex_alloc_state(env);
  g722_1_decode_state_t decoder_state;
  state->decode_state = g722_1_decode_init(&decoder_state, bit_rate, sample_rate);

  if (state->decode_state == NULL) {
    res = create_result_error(env, "decode_init");
    goto exit_create;
  }

  res = create_result_ok(env, state);
exit_create:
  unifex_release_state(env, state);
  return res;
}

UNIFEX_TERM create(UnifexEnv *env, int bit_rate, int sample_rate) {
  UNIFEX_TERM res;

  State *state = unifex_alloc_state(env);
  g722_1_decode_state_t decoder_state;
  state->decode_state = g722_1_decode_init(&decoder_state, bit_rate, sample_rate);

  if (state->decode_state == NULL) {
    res = create_result_error(env, "decode_init");
    goto exit_create;
  }

  res = create_result_ok(env, state);
exit_create:
  unifex_release_state(env, state);
  return res;
}

and now is working correctly. I'm new to C programming, reread about pointers and it seems clear for me now. but may be I fixed it incorrectly, thanks for directions btw final fix:

UNIFEX_TERM create(UnifexEnv *env, int bit_rate, int sample_rate) {
  UNIFEX_TERM res;

  State *state = unifex_alloc_state(env);

  state->decode_state = g722_1_decode_init(NULL, bit_rate, sample_rate);

  if (state->decode_state == NULL) {
    res = create_result_error(env, "decode_init");
    goto exit_create;
  }

  if (state->decode_state == NULL) {
    res = create_result_error(env, "decode_init");
    goto exit_create;
  }

  res = create_result_ok(env, state);
exit_create:
  unifex_release_state(env, state);
  return res;
}

UNIFEX_TERM create(UnifexEnv *env, int bit_rate, int sample_rate) {
  UNIFEX_TERM res;

  State *state = unifex_alloc_state(env);

  state->decode_state = g722_1_decode_init(NULL, bit_rate, sample_rate);

  if (state->decode_state == NULL) {
    res = create_result_error(env, "decode_init");
    goto exit_create;
  }

  if (state->decode_state == NULL) {
    res = create_result_error(env, "decode_init");
    goto exit_create;
  }

  res = create_result_ok(env, state);
exit_create:
  unifex_release_state(env, state);
  return res;
}

If I put NULL as initial parameter - g7221_1_decode_init initializes memory itself and it is correctly freed on destroy

Qizot•9mo ago

glad it works 🙂

Gaming

Programming

unifex seg fault on handle_destroy_state