Выделенная функция для выделения памяти вызывает утечку памяти?

Question

Hy all,

Я полагаю, что следующий фрагмент кода вызывает утечку памяти?

    /* External function to dynamically allocate a vector */
    template <class T>
            T *dvector(int n){
            T *v;

            v = (T *)malloc(n*sizeof(T));

            return v;
    }


    /* Function that calls DVECTOR and, after computation, frees it */
    void DiscontinuousGalerkin_Domain::computeFaceInviscidFluxes(){
            int e,f,n,p;
            double *Left_Conserved;

            Left_Conserved = dvector<double>(NumberOfProperties);

            //do stuff with Left_Conserved
            //

            free(Left_Conserved);

            return;
    }

Я думал, что, передав указатель в DVECTOR, он выделит его и вернет правильный адрес, так что free (Left_Conserved) будет успешно освобожден. Однако, похоже, это не так.

ПРИМЕЧАНИЕ: Я также проверил с new / delete , заменив malloc / free , но также безуспешно.

У меня есть похожий кусок кода для выделения двумерного массива. Я решил так управлять векторами / массивами, потому что я часто их использую, и я также хотел бы понять немного более глубокое управление памятью с C ++.

Итак, я бы очень хотел сохранить внешнюю функцию для распределения векторов и массивов для меня. В чем прикол, чтобы избежать утечки памяти?

EDIT

Я также использовал функцию DVECTOR для выделения пользовательских типов, так что это, вероятно, проблема, так как у меня нет вызываемых конструкторов.

Несмотря на то, что в фрагменте кода до того, как я освобожу вектор Left_Conserved, я также хотел бы иначе выделить вектор и оставить его «открытым» для оценки через его указатель другими функциями. Если использовать BOOST, он автоматически очистит выделение в конце функции, поэтому я не получу «публичный» массив с BOOST, верно? Я полагаю, что это легко исправить с помощью NEW, но что может быть лучше для матрицы?

Мне только что пришло в голову, что я передаю указатель в качестве аргумента другим функциям. Теперь BOOST, похоже, не очень этим наслаждается, и компиляция завершается с ошибками.

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

Вот что я хотел бы сделать:

    template <class T>
    T **dmatrix(int m, int n){
            T **A;

            A = (T **)malloc(m*sizeof(T *));
            A[0] = (T *)malloc(m*n*sizeof(T));

            for(int i=1;i<m;i++){
                    A[i] = A[i-1]+n;
            }

            return A;
    }


    void Element::setElement(int Ptot, int Qtot){

            double **MassMatrix;

            MassMatrix = dmatrix<myT>(Ptot,Qtot);

            FillInTheMatrix(MassMatrix);

            return;
    }

Answer 1

Там нет утечки памяти, но вы должны использовать new / delete [] вместо malloc / free. Тем более, что ваша функция является шаблонной.

Если вы когда-нибудь захотите использовать тип с нетривиальным конструктором, ваша функция на основе malloc не работает, поскольку она не вызывает никаких конструкторов.

Я бы заменил "dvector" на следующее:

void DiscontinuousGalerkin_Domain::computeFaceInviscidFluxes(){
        double *Left_Conserved = new double[NumberOfProperties];

        //do stuff with Left_Conserved
        //

        delete[] Left_Conserved;
}

Он функционально эквивалентен (за исключением того, что он может вызывать конструкторы для других типов). Это проще и требует меньше кода. Кроме того, каждый программист на С ++ сразу узнает, что происходит, поскольку в нем нет дополнительной функции.

Еще лучше, используйте умные указатели, чтобы полностью избежать утечек памяти:

void DiscontinuousGalerkin_Domain::computeFaceInviscidFluxes(){
        boost::scoped_array<double> Left_Conserved(new double[NumberOfProperties]);

        //do stuff with Left_Conserved
        //
}

Как многие умные программисты любят говорить, «лучший код - это код, который вы не должны писать»

РЕДАКТИРОВАТЬ: Почему вы считаете, что код, который вы опубликовали утечки памяти?

РЕДАКТИРОВАТЬ: Я видел ваш комментарий к другому сообщению, говоря

При выполнении кода команда top показывает выделенная память растет на неопределенный срок!

Это может быть абсолютно нормальным (или не вполне) в зависимости от вашей схемы распределения. Обычно способ работы кучи состоит в том, что они часто растут, но не часто сжимаются (это способствует последующему распределению). Полностью симметричные распределения и освобождения должны позволить приложению стабилизироваться при определенном объеме использования.

Например:

while(1) {
    free(malloc(100));
}

не должно приводить к непрерывному росту, потому что куча, скорее всего, даст один и тот же блок для каждого malloc.

Так что мой вопрос к вам. Растет ли он «бесконечно» или просто не сжимается?

EDIT:

Вы спросили, что делать с 2D-массивом. Лично я бы использовал класс, чтобы обернуть детали. Я бы либо использовал библиотеку (я считаю, что в boost есть класс массива с n-мерной размерностью), либо переход на собственную не должен быть слишком сложным. Что-то вроде этого может быть достаточно:

http://www.codef00.com/code/matrix.h

Использование происходит так:

Matrix<int> m(2, 3);
m[1][2] = 10;

Технически более эффективно использовать что-то вроде operator () для индексации класса-оболочки матрицы, но в этом случае я решил симулировать синтаксис собственного массива. Если эффективность действительно важна, ее можно сделать такой же эффективной, как и собственные массивы.

РЕДАКТИРОВАТЬ: другой вопрос. На какой платформе вы разрабатываете? Если это * nix, то я бы порекомендовал valgrind, чтобы помочь выявить утечку памяти. Поскольку предоставленный вами код явно не является проблемой.

Я не знаю ни одного, но я уверен, что в Windows также есть инструменты профилирования памяти.

РЕДАКТИРОВАТЬ: для матрицы, если вы настаиваете на использовании простых старых массивов, почему бы просто не выделить ее как один непрерывный блок и выполнить простую математику при индексации следующим образом:

T *const p = new T[width * height];

затем, чтобы получить доступ к элементу, просто сделайте это:

p[y * width + x] = whatever;

таким образом вы делаете delete[] для указателя, будь то массив 1D или 2D.

Answer 2

Нет видимой утечки памяти, однако существует высокий риск утечки памяти с таким кодом. Старайтесь всегда заключать ресурсы в объект (RAII). std :: vector делает именно то, что вы хотите:

void DiscontinuousGalerkin_Domain::computeFaceInviscidFluxes(){
        int e,f,n,p;
        std::vector<double> Left_Conserved(NumOfProperties);//create vector with "NumOfProperties" initial entries

        //do stuff with Left_Conserved
        //exactly same usage !
        for (int i = 0; i < NumOfProperties; i++){//loop should be "for (int i = 0; i < Left_Conserved.size();i++)" .size() == NumOfProperties ! (if you didn't add or remove any elements since construction
             Left_Conserved[i] = e*f + n*p*i;//made up operation
        }
        Left_Conserved.push_back(1.0);//vector automatically grows..no need to manually realloc
        assert(Left_Conserved.size() == NumOfProperties + 1);//yay - vector knows it's size
        //you don't have to care about the memory, the Left_Conserved OBJECT will clean it up (in the destructor which is automatically called when scope is left)
        return;
}

РЕДАКТИРОВАТЬ: добавлено несколько примеров операций. Вы действительно должны прочитать о STL-контейнерах, они того стоят!
РЕДАКТИРОВАТЬ 2: для 2d вы можете использовать:

std::vector<std::vector<double> >

как кто-то предложил в комментариях. но использование с 2d немного сложнее. Вы должны сначала заглянуть в 1-й случай, чтобы узнать, что происходит (увеличение векторов и т. Д.)

Answer 3

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

• Распределение с malloc моих пользовательских типов было ошибочным. Однако, когда я изменил его на new , утечка стала еще хуже, и это потому, что один из моих пользовательских типов имел конструктор, вызывающий внешнюю функцию без параметров и без правильного управления памятью. Поскольку я вызывал эту функцию после конструктора, в самой обработке не было ошибки, а только при распределении памяти. Так что new и правильный конструктор решили одну из основных утечек памяти.

• Другая утечка была связана с ошибочной командой освобождения памяти, которую я смог изолировать с помощью Valgrind (и немного терпения, чтобы правильно получить ее вывод). Итак, вот ошибка (и, пожалуйста, не называйте меня идиотом!):

  if (something){
          //do stuff
          return;    //and here it is!!!  =P
  }
  free();
  return;
  

И именно здесь RAII, как я понял, избежал неправильного программирования именно так. На самом деле я еще не изменил его на кодировку std :: vector или boost :: scoped_array, потому что мне все еще не ясно, может ли a передать их в качестве параметра другим функциям. Поэтому я все еще должен быть осторожен с delete [] .

В любом случае утечка памяти исчезла (сейчас ...) = D

Answer 4

Что произойдет, если вы передадите отрицательное значение для n на dvector?

Возможно, вам следует подумать об изменении сигнатуры функции, чтобы в качестве аргумента использовался тип без знака:

template< typename T >
T * dvector( std::size_t n );

Кроме того, в качестве стиля я предлагаю всегда предоставлять собственную функцию освобождения памяти каждый раз, когда вы предоставляете функцию выделения памяти. Как и сейчас, вызывающие абоненты полагаются на знание, что dvector реализован с использованием malloc (и что free является соответствующим вызовом освобождения). Примерно так:

template< typename T >
void dvector_free( T * p ) { free( p ); }

Как и другие предлагали, сделать это как класс RAII было бы более надежным. И, наконец, как уже предлагали другие, для этого существует множество проверенных временем библиотек, поэтому вам, возможно, даже не понадобится создавать свои собственные.

Answer 5

Нет, если вы не делаете ничего радикального между вызовом вашего dvector шаблона и free, вы не теряете память. Что говорит вам, что есть утечка памяти?

Могу я спросить, почему вы решили создавать свои собственные массивы вместо использования контейнеров STL, таких как vector или list? Это, безусловно, спасло бы вас от многих проблем.

Answer 6

Я не вижу утечки памяти в этом коде.

Если вы пишете программы на c ++ - используйте new / delete вместо malloc / free.

Во избежание возможных утечек памяти используйте интеллектуальные указатели или контейнеры stl.