Есть ли здесь вероятность утечки ресурсов? - PullRequest
2 голосов
/ 20 марта 2011

Следующий пример (не скомпилированный, поэтому я не буду ручаться за синтаксис) извлекает два ресурса из пулов ресурсов (не выделенных с помощью new), а затем «связывает» их вместе с MyClass на время определенной транзакции.

Транзакция, реализованная здесь myFunc, пытается защитить от утечки этих ресурсов путем отслеживания их «владения».Указатели локальных ресурсов очищаются, когда очевидно, что создание экземпляра MyClass прошло успешно.Локальный catch, а также деструктор ~MyClass возвращают ресурсы в их пул (двойные освобождения защищены вышеупомянутой очисткой локальных указателей).

Создание экземпляра MyClass может завершиться неудачно и привести кв исключении в два этапа (1) фактическое выделение памяти или (2) в самом теле конструктора.У меня нет проблем с № 1, но в случае № 2, если выбрасывается исключение ПОСЛЕ m_resA & m_resB были установлены.Принятие и 1011 * и кода очистки myFunc для принятия ответственности за возврат этих ресурсов в их пулы.

Это разумная проблема?

Опции, которые я рассмотрел, но нене нравится:

  • Умные указатели (например, Boost's shared_ptr).Я не видел, как применить к пулу ресурсов (за исключением переноса в еще один экземпляр).
  • Разрешение двойного освобождения на этом уровне, но защита в пулах ресурсов.
  • Попытка использовать тип исключения - попытка сделать вывод, что если bad_alloc было обнаружено, MyClass не стал владельцем.Это потребует использования try-catch в конструкторе, чтобы убедиться, что любые ошибки выделения в ABC() ...more code here... не будут перепутаны с ошибками выделения MyClass.

Есть ли простое простое решение, которое я пропустил?

class SomeExtResourceA;
class SomeExtResourceB;

class MyClass {
private:
  // These resources come out of a resource pool not allocated with "new" for each use by MyClass
  SomeResourceA* m_resA;
  SomeResourceB* m_resB;

public:
  MyClass(SomeResourceA* resA, SomeResourceB* resB):
    m_resA(resA), m_resB(resB)
    {
       ABC(); // ... more code here, could throw exceptions
    }

  ~MyClass(){
    if(m_resA){
      m_resA->Release();
    }
    if(m_resB){
      m_resB->Release();
    }
  }
};

void myFunc(void)
{
  SomeResourceA* resA    = NULL;
  SomeResourceB* resB    = NULL;
  MyClass*       pMyInst = NULL;

  try {
    resA = g_pPoolA->Allocate();
    resB = g_pPoolB->Allocate();
    pMyInst = new MyClass(resA,resB);
    resA=NULL; // ''ownership succesfully transfered to pMyInst
    resB=NULL; // ''ownership succesfully transfered to pMyInst

    // Do some work with pMyInst;
    ...;

    delete pMyInst;

  } catch (...) {
    // cleanup
    // need to check if resA, or resB were allocated prior 
    // to construction of pMyInst.
    if(resA) resA->Release();
    if(resB) resB->Release();
    delete pMyInst;
    throw; // rethrow caught exception
  }
}

Ответы [ 5 ]

3 голосов
/ 20 марта 2011

Вот ваш шанс выпустить двойной звонок:

void func()
{
   MyClass   a(resourceA, resourceB);
   MyClass   b(a);
}

Упс.

Если вы используете оболочку RIAA для своих ресурсов, у вас будет гораздо меньше шансов ошибиться.Это может привести к ошибкам.В настоящее время вам не хватает конструктора копирования и оператора присваивания в MyClass, которые потенциально могут привести к двойному вызову Release (), как показано выше.

Из-за сложности обработки ресурса класс должен владеть только одним ресурсом.Если у вас есть несколько ресурсов, делегируйте их право собственности классу, который он выделил для их владения, и используйте несколько таких объектов в вашем классе.

Редактировать 1

Давайте сделаем некоторые предположения:

Ресурсы распределяются и подсчитываются.Вы увеличиваете счет с помощью Acquire () и уменьшаете счет с помощью Release ().Когда счетчик достигает нуля, они автоматически уничтожаются.

class ReferenceRapper
{ 
    ReferenceBase*   ref;
    public:
        ReferenceWrapper(ReferenceBase* r) : ref (r)  {/* Pool set the initial count to 1 */ }
       ~ReferenceWrapper()                            { if (ref) { ref->Release();} }

        /*
         * Copy constructor provides strong exception guarantee (aka transactional guarantee)
         * Either the copy works or both objects remain unchanged.
         *
         * As the assignment operator is implemented using copy/swap it also provides
         * the strong exception guarantee.
         */
        ReferenceWrapper(ReferenceWrapper& copy)
        {
            if (copy.ref) {copy.ref->Acquire();}
            try
            {
                if (ref) {ref->Release();}
            }
            catch(...)
            {
                if (copy.ref)
                {  copy.ref->Release(); // old->Release() threw an exception. 
                                        // Must reset copy back to its original state.
                }
                throw;
            }
            ref = copy.ref;
        }
        /* 
         * Note using the copy and swap idium.
         * Note: To enable NRVO optimization we pass by value to make a copy of the RHS.
         *       rather than doing a manual copy inside the method.
         */
        ReferenceWrapper& operator(ReferenceWrapper rhsCopy)
        {
            this->swap(rhsCopy);
        }
        void swap(ReferenceWrapper& rhs) throws ()
        {
            std::swap(ref, rhs.ref);
        }
        // Add appropriate access methods like operator->()
};

Теперь, когда тяжелая работа выполнена (управление ресурсами).Реальный код становится тривиальным для записи.

class MyClass
{
        ReferenceWrapper<SomeResourceA>  m_resA;
        ReferenceWrapper<SomeResourceB>  m_resB;
    public:
        MyClass(ReferenceWrapper<SomeResourceA>& a, ReferenceWrapper<SomeResourceB>& b)
            : m_resA(a)
            , m_resB(b)
        {
           ABC();
        }
};

void myFunc(void)
{
  ReferenceWrapper<SomeResourceA> resA(g_pPoolA->Allocate());
  ReferenceWrapper<SomeResourceB> resB(g_pPoolB->Allocate());

  std::auto_ptr<MyClass>         pMyInst = new MyClass(resA, resB);


  // Do some work with pMyInst;
}

Редактировать 2 Исходя из комментария ниже, ресурсы имеют только одного владельца:

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

  1. Удалите метод Release () и сделайте всю работу в деструкторе.
  2. Измените методы пула, чтобы указатель создавался в std :: auto_ptrи вернуть std :: auto_ptr.

Код:

class MyClass
{
        std::auto_ptr<SomeResourceA>  m_resA;
        std::auto_ptr<SomeResourceB>  m_resB;
    public:
        MyClass(std::auto_ptr<SomeResourceA>& a, std::auto_ptr<SomeResourceB>& b)
            : m_resA(a)
            , m_resB(b)
        {
           ABC();
        }
};

void myFunc(void)
{
  std::auto_ptr<SomeResourceA> resA(g_pPoolA->Allocate());
  std::auto_ptr<SomeResourceB> resB(g_pPoolB->Allocate());

  std::auto_ptr<MyClass>       pMyInst = new MyClass(resA, resB);


  // Do some work with pMyInst;
}
1 голос
/ 20 марта 2011

Ваш код в порядке. Но чтобы сделать его еще лучше, используйте какой-нибудь умный указатель!

Редактировать: например, вы можете использовать shared_ptr:

class SomeExtResourceA;
class SomeExtResourceB;

class MyClass {
private:
  // These resources come out of a resource pool not allocated with "new" for each use by MyClass
  shared_ptr<SomeResourceA> m_resA;
  shared_ptr<SomeResourceB> m_resB;

public:
  MyClass(const shared_ptr<SomeResourceA> &resA, const shared_ptr<SomeResourceB> &resB):
    m_resA(resA), m_resB(resB)
    {
       ABC(); // ... more code here, could throw exceptions
    }
  }
};

void myFunc(void)
{
  shared_ptr<SomeResourceA> resA(g_pPoolA->Allocate(), bind(&SomeResourceA::Release, _1));
  shared_ptr<SomeResourceB> resB(g_pPoolB->Allocate(), bind(&SomeResourceB::Release, _1));
  MyClass pMyInst(resA,resB);

  // you can reset them here if you want, but it's not necessery:
  resA.reset(), resB.reset();

  // use pMyInst
}

Я считаю, что это решение с RAII намного проще.

1 голос
/ 20 марта 2011

Я не вижу утечки в этом небольшом коде.

Если конструктор выдает исключение, то деструктор не будет вызван, поскольку объект никогда не существовал.Следовательно, я также не вижу двойного удаления!

Из этой статьи Херба Саттера: Исключения конструктора в C ++, C # и Java :

  • конструкторконцептуально превращает кусок необработанной памяти подходящего размера в объект, который подчиняется своим инвариантам. Время жизни объекта не начинается, пока его конструктор не завершится успешно .Если конструктор заканчивается созданием исключения, это означает, что он так и не завершил создание объекта и установку его инвариантов - и в момент выхода из исключительного конструктора, объект не только не существует, но и никогда не существовал.
  • Деструктор / распределитель концептуально превращает объект обратно в необработанную память.Поэтому, как и все другие непривилегированные методы, деструкторы / диспозиторы предполагают в качестве предварительного условия, что этот объект на самом деле является допустимым объектом и что его инварианты сохраняются.Следовательно, деструкторы / утилизаторы работают только на успешно построенных объектах.

Я думаю, это должно рассеять ваши сомнения!

0 голосов
/ 20 марта 2011

Классическое использование для явного владения - это std :: auto_ptr

Примерно так:

std::auto_ptr<SomeResourceA>(g_pPoolA->Allocate()) resA;
std::auto_ptr<SomeResourceB>(g_pPoolB->Allocate()) resB;
pMyInst = new MyClass(resA.release(),resB.release());

Передача прав собственности при вызове конструктора.

0 голосов
/ 20 марта 2011

Просто поместите if (pMyInst) { ... } вокруг кода освобождения / удаления в вашем улове, и все в порядке.

...