Firebird и C++

Привет всем.

Вследствие переезда SVN-репозитория Firebird (теперь он доступен по адресу http://svn.code.sf.net/p/firebird/code), пришлось заново его выкачивать — я не стал разбираться с тем, как перебиндить мою текущую локальную копию к новому репозитарию. После решил посмотреть содержимое пары случайных исходных файлов. В качестве отправной точки для выборки взял результаты поиска слов «CreateEvent» и «addref».

Комментировать код, наверное, нет смысла. Оно и так все понятно…

CreateEvent
файл «src/common/classes/conditional.h»

class Condition
{
 private:
  //...
  HANDLE events[MAX_EVENTS];

  void init()
  {
   events[SIGNAL] = CreateEvent(NULL, FALSE, FALSE, NULL);
   events[BROADCAST] = CreateEvent(NULL, TRUE, FALSE, NULL);

   if (!events[SIGNAL] || !events[BROADCAST])
    system_call_failed::raise("CreateCondition(Event)");
  }

 public:
  Condition() { init(); }
  explicit Condition(MemoryPool&) { init(); }

 ~Condition()
  {
   if (events[SIGNAL] && !CloseHandle(events[SIGNAL]))
    system_call_failed::raise("CloseHandle(SIGNAL)");
   
   if (events[BROADCAST] && !CloseHandle(events[BROADCAST]))
    system_call_failed::raise("CloseHandle(BROADCAST)");
  }
  //...

AddRef
файл «src/yvalve/DistributedTransaction.cpp»

YTransaction* FB_CARG Dtc::start(IStatus*     status,
                                 unsigned int cnt,
                                 DtcStart*    components)
{
 try
 {
  status->init();
  RefPtr<DTransaction> dtransaction(new DTransaction);

  //...
  dtransaction->addRef();
  return new YTransaction(NULL, dtransaction);
 }
 catch (const Exception& ex)
 {
  ex.stuffException(status);
 }
 return NULL;
}

Dmitry Kovalenko on 27 января, 2013 | Filed Under Firebird Server, Программирование

8 комментариев

Alex  on 31 января, 2013

Дима, если ты хошь достучаться до аффтара — надо писать по бразильски (ну в крайнем случеа по английски). И не здесь 🙂

Kovalenko  on 31 января, 2013

Привет.

Ну …

Первое творчество приехало из CVS — «(roman-si 17-Mar-09)»

Во втором случае, бразилец (скорее всего) всего лишь жертва корявого дизайна. YTransaction должен сам блокировать переданные объекты. И если быть совсем точным — это должен делать не он, а принадлежащий ему смарт-указатель.

Alex  on 1 февраля, 2013

Насчёт первого — я уже увы настолько отошёл от вендо-специфик что не могу оценить кривизну. Если ты про то что игнорируется пул памяти в классе, так это не есть проблема.

Во втором случае всё и правда прикольней. YTransaction блокирует переданный ему объект, и (более того) там внутри живёт smart-указатель. Но он ожидает получить объект не из к-ра, а от функции вида
ITransaction* OtherClass::startTransaction(…)
откуда объект приходит уже с доп. addRef(). Может оно и кривой дизайн, но вроде робыть.

Kovalenko  on 1 февраля, 2013

Привет.

Генерация исключения в деструкторе глаза не режет?

Потенциальная утечка ресурса в init — тоже ничего страшного?

Ну и классика — отсутствие запрета на генерацию конструктора и оператора копирования по-умолчанию.

Не слишком ли много проблем для такого тривиального класса?

——
По-поводу кривого дизайна с AddRef….

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

Kovalenko  on 2 февраля, 2013

Меня тут посетила мысль, что проблема c AddRef (как и в первом случае) так и не была осознана. Поэтому, на всякий случай, все таки объясню.

dtransaction->addRef();
return new YTransaction(NULL, dtransaction);

Если при выделении памяти для YTransaction или его конструировании произойдет исключение, то dtransaction скорее всего не будет разблокирован. Что, по всей видимости, приведет к утечке памяти.

dimitr  on 3 марта, 2013

> Генерация исключения в деструкторе глаза не режет?
> Потенциальная утечка ресурса в init — тоже ничего страшного?

В обоих случаях — ответ скорее всего «все ОК». Класс исключений system_call_failed достаточно специфичный, он подразумевает настолько поганую ситуацию, после которой сервер уже полноценно работать не сможет. В DEV_BUILD это сразу приводит к аборту. Релизный билд пытается выжить, но в реальной жизни у него вряд ли это получится. В таких условиях лично меня мало волнует утечка ресурсов 🙂 Так что, с учетом вышесказанного, выброс именно этого исключения в деструкторе — вполне допустим. Хотя лично я бы предпочел, чтобы выброс system_call_failed и в релизном билде завершал сервер, только чуть поприличнее чем аборт.

> отсутствие запрета на генерацию конструктора и оператора копирования по-умолчанию

Это явный косяк, причем довольно редкий в наших классах. Поправим.

> проблема c AddRef

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

Kovalenko  on 4 марта, 2013

В первом случае
1. Конструктор должен генерировать исключение. Причем не конструктор Condition, а конструктор специального класса, который управляет дескриптором события.

2. Деструктор этого специального класса для дескриптора не будет вызываться, если конструктор не отработал до конца (событие не было создано). А если он вызвался и получил ошибку с вызовом CloseHandle, то в отладочной версии он может заюзать ассерт. А в релизной он вообще ничего не должен делать. Потому что скорее всего к процессу пришел пушистый зверь и исключение может окончательно вывести его из себя.

Это классика.

Насчет второго случая.

Это все работает до тех пор пока в объекты в пуле не содержат указатели и дескрипторы на внешние ресурсы. Или у вас такой мудрый пул, что умеет вызывать деструкторы? Я в этом сильно сомневаюсь.

У вас уже есть отличный баг с управлением внешних ресурсов, который приводит к картинке такого вида


Состояние процесса FB 2.5.3.26543 SuperClassic x64 [win]
Утечка памяти в сервере при работе с кодовыми страницами ICU.

А вы еще планируете внешние хранимые процедуры, триггеры, другие приблуды.

Kovalenko  on 4 марта, 2013

Вспомнилось из недавно прочитанного в девеле

Давайте привлекать студентов. Пусть изучают как написан сервер баз данных.

Как он написан и работает они вряд ли поймут. Потому что у вас у самих нет людей которые это до конца понимают. Это нормально. Но увидят и возьмут на вооружение охинею подобную выше обозначенной.

Но это так. Еще одна маленькая ложка цинизма в этот чан с ….

Leave a Comment