Память не освобождается, что приводит к гигантской утечке памяти - PullRequest
1 голос
/ 02 мая 2010

К сожалению, решения пока не работают; установка указателя result.values ​​в 0 фактически не освобождает использование памяти. Я также попытался использовать free (result.values) вместо этого, но это нежелательно, поскольку это удаляет мою строку.

Редактировать 2: Я попробую написать деструктор стека.

Редактировать 3: Попался. Спасибо DeadMG, написание деструктора, который бесплатно (значения) сделал свое дело отлично! Ого ... так просто.

В моей библиотеке Unicode для C ++ класс ustring имеет функции operator =, установленные для значений char * и других значений ustring. При выполнении простого теста на утечку памяти:

#include <cstdio>
#include "ucpp"
main() {
  ustring a;
  for(;;)a="MEMORY";
}

память, используемая программой, растет неуправляемо (характерно для программы с большой утечкой памяти), хотя я добавил вызовы free () для обеих функций. Я не уверен, почему это неэффективно (я пропускаю вызовы free () в других местах?)

Это текущий код библиотеки:

#include <cstdlib>
#include <cstring>
class ustring {
  int * values;
  long len;
  public:
  long length() {
    return len;
  }
  ustring() {
    len = 0;
    values = (int *) malloc(0);
  }
  ustring(const ustring &input) {
    len = input.len;
    values = (int *) malloc(sizeof(int) * len);
    for (long i = 0; i < len; i++)
      values[i] = input.values[i];
  }
  ustring operator=(ustring input) {
    ustring result(input);
    free(values);
    len = input.len;
    values = input.values;
    return * this;
  }
  ustring(const char * input) {
    values = (int *) malloc(0);
    long s = 0;                                                                 // s = number of parsed chars
    int a, b, c, d, contNeed = 0, cont = 0;
    for (long i = 0; input[i]; i++)
      if (input[i] < 0x80) {                                                    // ASCII, direct copy (00-7f)
        values = (int *) realloc(values, sizeof(int) * ++s);
        values[s - 1] = input[i];
      } else if (input[i] < 0xc0) {                                             // this is a continuation (80-bf)
        if (cont == contNeed) {                                                 // no need for continuation, use U+fffd
          values = (int *) realloc(values, sizeof(int) * ++s);
          values[s - 1] = 0xfffd;
        }
        cont = cont + 1;
        values[s - 1] = values[s - 1] | ((input[i] & 0x3f) << ((contNeed - cont) * 6));
        if (cont == contNeed) cont = contNeed = 0;
      } else if (input[i] < 0xc2) {                                             // invalid byte, use U+fffd (c0-c1)
        values = (int *) realloc(values, sizeof(int) * ++s);
        values[s - 1] = 0xfffd;
      } else if (input[i] < 0xe0) {                                             // start of 2-byte sequence (c2-df)
        contNeed = 1;
        values = (int *) realloc(values, sizeof(int) * ++s);
        values[s - 1] = (input[i] & 0x1f) << 6;
      } else if (input[i] < 0xf0) {                                             // start of 3-byte sequence (e0-ef)
        contNeed = 2;
        values = (int *) realloc(values, sizeof(int) * ++s);
        values[s - 1] = (input[i] & 0x0f) << 12;
      } else if (input[i] < 0xf5) {                                             // start of 4-byte sequence (f0-f4)
        contNeed = 3;
        values = (int *) realloc(values, sizeof(int) * ++s);
        values[s - 1] = (input[i] & 0x07) << 18;
      } else {                                                                  // restricted or invalid (f5-ff)
        values = (int *) realloc(values, sizeof(int) * ++s);
        values[s - 1] = 0xfffd;
      }
    len = s;
  }
  ustring operator=(const char * input) {
    ustring result(input);
    free(values);
    len = result.len;
    values = result.values;
    return * this;
  }
  ustring operator+(ustring input) {
    ustring result;
    result.len = len + input.len;
    result.values = (int *) malloc(sizeof(int) * result.len);
    for (long i = 0; i < len; i++)
      result.values[i] = values[i];
    for (long i = 0; i < input.len; i++)
      result.values[i + len] = input.values[i];
    return result;
  }
  ustring operator[](long index) {
    ustring result;
    result.len = 1;
    result.values = (int *) malloc(sizeof(int));
    result.values[0] = values[index];
    return result;
  }
  operator char * () {
    return this -> encode();
  }
  char * encode() {
    char * r = (char *) malloc(0);
    long s = 0;
    for (long i = 0; i < len; i++) {
      if (values[i] < 0x80)
        r = (char *) realloc(r, s + 1),
        r[s + 0] = char(values[i]),
        s += 1;
      else if (values[i] < 0x800)
        r = (char *) realloc(r, s + 2),
        r[s + 0] = char(values[i] >> 6 | 0x60),
        r[s + 1] = char(values[i] & 0x3f | 0x80),
        s += 2;
      else if (values[i] < 0x10000)
        r = (char *) realloc(r, s + 3),
        r[s + 0] = char(values[i] >> 12 | 0xe0),
        r[s + 1] = char(values[i] >> 6 & 0x3f | 0x80),
        r[s + 2] = char(values[i] & 0x3f | 0x80),
        s += 3;
      else
        r = (char *) realloc(r, s + 4),
        r[s + 0] = char(values[i] >> 18 | 0xf0),
        r[s + 1] = char(values[i] >> 12 & 0x3f | 0x80),
        r[s + 2] = char(values[i] >> 6 & 0x3f | 0x80),
        r[s + 3] = char(values[i] & 0x3f | 0x80),
        s += 4;
    }
    return r;
  }
};

Ответы [ 5 ]

13 голосов
/ 02 мая 2010

Где деструктор из ustring? Разве это не должно освободить выделенную память?


Давайте посмотрим, например, у оператора присваивания:

ustring operator=(ustring input)
{
    ustring result(input);
    ...
    return *this;
}

Вы передаете ustring параметр по значению. Это, вероятно, приведет к тому, что копия будет создана с помощью конструктора копирования. Вы вызываете конструкцию копии в другой раз для инициализации result. Я подозреваю, что вы вызываете конструкцию копирования еще раз, когда возвращаете *this как ustring (снова по значению, а не по ссылке).

Давайте рассмотрим один из этих трех случаев, а именно result: когда эта локальная переменная выходит из области видимости (т.е. в конце функции), она будет автоматически уничтожена. Но если вы не предоставите деструктор (~ustring), который free выделяет выделенную память, вы получите утечку памяти.

И так как у вас, очевидно, есть множество операций копирования и передачи по значению без деструктора для освобождения выделенной памяти, вы получите много-много-много несвободной памяти.


Кроме того: Почему вы используете malloc и free вместо new[] и delete[]? Вы можете избавиться от уродливых вычислений sizeof(int) * ... и приведений типа (int*). Вместо:

values = (int *) malloc(sizeof(int) * len);

вы просто напишите:

values = new int[len];
4 голосов
/ 02 мая 2010

По сути, вы создаете слишком много строк, вам нужно намного больше ссылок, и вы не реализовали деструктор, поэтому, когда они все падают со стека, они не освобождаются.

Также, когда в вашем операторе присваивания вам нужно установить значение result.values ​​в NULL, иначе память будет удалена. Вы можете использовать оператор перемещения, чтобы сделать эту быструю операцию приятной, хотя я до сих пор не понимаю, почему вы это сделали.

2 голосов
/ 02 мая 2010

Как только Вы внедрите деструктор, эта реализация назначения укусит Вас

  ustring operator=(const char * input) {
    ustring result(input);
    free(values);
    len = result.len;
    values = result.values;
    return * this;
  }

Нет, это не аргумент для того, чтобы оставить деструктор не реализованным.

EDIT:

Объекту ustring принадлежит массив, выделенный с помощью malloc, и он должен освободить эту память, когда он будет уничтожен.

Если вы создаете локальный объект

    ustring result(input);

Он будет уничтожен, когда функция вернется.

В строке

    values = result.values;

Вы копируете указатель из вашего локального объекта - результат, но результат не знает об этом, поэтому он должен уничтожить массив и оставить его с висящим указателем. Вы должны изменить состояние результата таким образом, чтобы предотвратить освобождение памяти, поскольку вы взяли на себя владение массивом. Одним из способов сделать это может быть установка указателя на ноль. Вызов free для нулевого указателя является законным, поэтому вам не нужно беспокоиться о попытке освободить память деструктором результата.

ustring& operator=(const char * input) {
    ustring result(input);
    free(values);
    len = result.len;
    values = result.values;
    result.values = 0;
    return * this;
}
2 голосов
/ 02 мая 2010
  ustring operator=(ustring input) {
    ustring result(input);
    free(values);
    len = input.len;
    values = input.values;
    return * this;
  }

Почему в этом объявлено result

0 голосов
/ 02 мая 2010
  1. Попробуйте использовать RAII-методы , чтобы избежать проблем с утечками памяти. Если вы оберните свои массивы чем-то вроде boost::shared_array, вам не придется писать явный деструктор, чтобы освободить память.
  2. ustring operator=(ustring input) должно быть ustring operator=(const ustring & input), чтобы избежать копирования аргумента. Это всегда следует делать, если вы передаете неинтегральный тип в качестве параметра только для чтения.
  3. Использование shared_array - это нечто вроде того, что избавляет от проблемы вычисления размера.

Просто несколько подсказок. С другой стороны, ваш код невероятно сложен для чтения и понимания. Вы должны действительно что-то сделать с этим, если вы ожидаете, что другие люди когда-либо будут работать с этим. Сделайте ваши имена более наглядными, рефакторинг повторяет код в функции, которые выражают то, что он делает. Например, вместо использования for -loop для копирования ваших значений, вы можете использовать std::copy. Вы также можете заменить все свои магические числа (например, 0x1f) на константы, имена которых выражают семантику значения.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...