Коротко. О "частично инициализированных объектах" (или опять о фабриках)

По мотивам:

Коротко. О конструкторах
Депрессия. Или превратности hResult и прочих ErrorCode
Коротко. И ещё о фабриках
Коротко. Ещё о фабриках
Коротко. О фабриках
Коротко. "Почему нужны тесты"
Фабричный метод

Сегодня таки (надеюсь) нашёл ошибку, которая "не давала жить" многие годы.

И следы которой мы наблюдали во всяких разных логах.

Но мы долго не могли её "опространствить".

Но сегодня "я поймал её за хвост". Под отладчиком.

И как ни банально -дело во всё тех же "частично инициализированных объектах".

Вот примерно так (код естественно - по-максимуму упрощаю):

construstor TStorageHeader.Create(aPosition: Int64);
begin
 Assert(aPosition >= 0);
 inherited Create;
 FillChar(f_Data, SizeOf(f_Data), 0); // - инициализируем данные
 f_Position := aPosition;
 LockFile(f_Position, SizeOf(f_Data)); // - тут блокируем регион файла и там может ПОДНИМАТЬСЯ ИСКЛЮЧЕНИЕ - LOCK_VIOLATION
 LoadData(f_Position, f_Data, SizeOf(f_Data)); // - тут читаем данные 
end;

destructor TStorageHeader.Destroy;
begin
 SaveData(f_Position, f_Data, SizeOf(f_Data)); // - тут записываем данные 
 UnLockFile(f_Position, SizeOf(f_Data)); // - тут разблокируем регион файла
 inherited;
end;

В чём тут проблема?

А вот в чём:

var
 l_H : TStorageHeader;
begin
 l_H := TStorageHeader.Create(aSomePosition);
 try
  ...
 finally
  FreeAndNil(l_H);
 end;//try..finally
end;

- если в конструкторе НЕТ ИСКЛЮЧЕНИЯ, то всё хорошо.

А вот если в конструкторе ЕСТЬ ИСКЛЮЧЕНИЕ, то АВТОМАТОМ вызовется деструктор.

На ЧАСТИЧНО ИНИЦИАЛИЗИРОВАННОМ объекте.

Что произойдёт? Если исключение было возбуждено в LockFile то код LoadData - не пройдёт.

Но! Деструктор - вызовется (читаем документацию).

Цитирую:

"Destructors called because of exceptions thrown from constructors"

И что произойдёт?

А то что в коде SaveData - сохранится НЕ ТО ЧТО СЧИТАЛИ, а то что инициализировали при помощи FillChar.

Проблема понятна?

Она - банальна. Но однако "жила многие годы". Почему? Потому что вероятность того что из LockFile "вылетит исключение" - была крайне низка. Потому что там блокировка происходит с учётом тайм-аута. Который для большинства операций - достаточен. Чтобы не летело исключение.

А более того - у нас был код (выше), который синхронизировал доступ к ресурсам на уровне "бизнес-логики".

Но!

Эта синхронизация негативно складывалась на производительности. То самое "бутылочное горлышко".

Клиенты "ждали друг друга" и не лезли к "общему ресурсу". Поэтому дело до LockFile который может "возбудить исключение" - не доходило. Просто - "все выстраивались в очередь" и работали последовательно, а не параллельно.

(Кстати вот что стоит почитать - Э. ДЕЙКСТРА ВЗАИМОДЕЙСТВИЕ ПОСЛЕДОВАТЕЛЬНЫХ ПРОЦЕССОВ. Хотя это "к делу не относится")

Но!

Мы разрулили это "бутылочное горлышко" и клиенты начали работать "по-настоящему параллельно и независимо".

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

А на автоматических тестах - тем более. Т.к. они работают без задержек и пауз, свойственных людям.

И ошибка стала появляться более стабильно и очень часто.

Особенно если запускать тесты под отладчиком и вставать на break-point'ах там "где надо". Эмулируя "задержки сети".

Что делать?

Ну в общем - "всё просто". Можно сделать например так:

construstor TStorageHeader.Create(aPosition: Int64);
begin
 Assert(aPosition >= 0);
 inherited Create;
 f_DataIsLoaded := false; // - для "надёжности"... а на самом деле, чтобы ПОСТУЛИРОВАТЬ наши намерения
 FillChar(f_Data, SizeOf(f_Data), 0); // - инициализируем данные
 f_Position := aPosition;
 LockFile(f_Position, SizeOf(f_Data)); // - тут блокируем регион файла и там может ПОДНИМАТЬСЯ ИСКЛЮЧЕНИЕ - LOCK_VIOLATION
 LoadData(f_Position, f_Data, SizeOf(f_Data)); // - тут читаем данные 
 f_DataIsLoaded := true; // - тут постулируем, что таки зачитали данные
end;

destructor TStorageHeader.Destroy;
begin
 if f_DataIsLoaded then // - пишем только то, что СЧИТАЛИ, а не "мусор"
  SaveData(f_Position, f_Data, SizeOf(f_Data)); // - тут записываем данные 
 UnLockFile(f_Position, SizeOf(f_Data)); // - тут разблокируем регион файла
 inherited;
end;

Ну или так:

construstor TStorageHeader.Create(aPosition: Int64);
begin
 Assert(aPosition >= 0);
 inherited Create;
 FillChar(f_Data, SizeOf(f_Data), 0); // - инициализируем данные
 f_Position := aPosition;
 LockFile(f_Position, SizeOf(f_Data)); // - тут блокируем регион файла и там может ПОДНИМАТЬСЯ ИСКЛЮЧЕНИЕ - LOCK_VIOLATION
 // - и там же "взводим флаг"- IsLocked
 LoadData(f_Position, f_Data, SizeOf(f_Data)); // - тут читаем данные 
end;

destructor TStorageHeader.Destroy;
begin
 if Self.IsLocked then // - пишем ТОЛЬКО если "заблокировали регион", а "стало быть" - и считали его
  SaveData(f_Position, f_Data, SizeOf(f_Data)); // - тут записываем данные 
 UnLockFile(f_Position, SizeOf(f_Data)); // - тут разблокируем регион файла
 inherited;
end;

Но "на мой вкус" - лучше сделать "фабричный метод".

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

construstor TStorageHeader.InternalCreate(aPosition: Int64);
begin
 inherited Create;
 FillChar(f_Data, SizeOf(f_Data), 0); // - инициализируем данные
 f_Position := aPosition;
 LoadData(f_Position, f_Data, SizeOf(TData)); // - тут читаем данные 
end;

class function TStorageHeader.Create(aPosition: Int64): TStorageHeader;
var
 l_H :TStorageHeader; 
begin
 Assert(aPosition >= 0); // - все ПРЕДУСЛОВИЯ переносим из КОНСТРУКТОРА в ФАБРИЧНЫЙ МЕТОД
 // Далее - ВЕСЬ код ПОТЕНЦИАЛЬНО поднимающий исключения переносим из конструктора в ФАБРИЧНЫЙ МЕТОД:
 LockManager.LockFile(aPosition, SizeOf(TData)); // - тут блокируем регион файла и там может ПОДНИМАТЬСЯ ИСКЛЮЧЕНИЕ - LOCK_VIOLATION
 // - если исключение поднимется, то до конструктора (а стало быть деструктора) - дело не дойдёт
 //   ну и "флажков" заводить не надо
 l_H := InternalCreate(aPosition);
 Result := l_H;
end;

destructor TStorageHeader.Destroy;
begin
 SaveData(f_Position, f_Data, SizeOf(f_Data)); // - тут записываем данные 
 UnLockFile(f_Position, SizeOf(f_Data)); // - тут разблокируем регион файла
 inherited;
end;

Ну и ещё стоит прочитать вот что - BeforeRelease.

Там описано - почему "сохраняющий код" надо выполнять раньше, чем собственно разрушение объекта.

Ну и по-моему - "очевидно", что сохранять надо "неразрушенный объект".

И стоит (на мой вкус) вынести весь "сохраняющий код" из деструктора в BeforeRelease (BeforeDestruction).

https://groups.google.com/forum/#!topic/borland.public.delphi.language.objectpascal/d7ajc3wkngY

Цитирую:

"I just stumbled upon some behaviour of Delphi, which I find pretty 
confusing: I prefer using "AfterConstruction" and "BeforeDestruction" 
for creating/destroying class members. Now, if an exception is raised in 
"AfterConstrucion", Delphi immediately destroys the object (ok so far) 
but "BeforeDestruction" is not called (not ok). Online help is not clear 
about this, but one could guess that it is *always* called. My 
conclusion is: always override "Destroy" instead of "BeforeDestruction" 
to safely clean up. Is that right? What's the use of "BeforeDestruction" 
then?"

На самом деле - is ok.

Читаем документацию:

http://docwiki.embarcadero.com/VCL/2010/en/System.TObject.BeforeDestruction

Цитирую:

"BeforeDestruction is called automatically before the object's first destructor executes. Do not call it explicitly in your applications."

"Note:  BeforeDestruction is not called when the object is destroyed before it is fully constructed. That is, if the object's constructor raises an exception, the destructor is called to dispose of the object, but BeforeDestruction is not called."

Может быть это всё и "витиевато" написанои "проблема высосана из пальца" (могу допустить такое), но буду рад, если вы это "переосмыслите"и найдёте подобные (непростые, хотя и банальные) ошибки у себя.

Ну если у вас их нет, то тем более - рад за вас :-)

Не хочу никого делать "насильно счастливыми". Ну а вдруг...

Ну а может быть - "руки у меня кривые" и подобные проблемы только у меня, но "это вряд ли".

P.S.Ну и ещё (Коротко. "Почему нужны тесты") - ошибка "жила" уже лет 15-ть как. "Портила кровь и нервы", но "не мешала жить".

И при этом - сотни (а то и тысячи) пользователей (внутренних) работали с этим.

Но!

Стоило только сделать тесты (Коротко. Сделали нагрузочные тесты) - как ошибка - "сразу вылезла наружу". Хотя она была найдена не на "сотнях клиентов", а на двух.. Повторю на двух... Двух, но ДЕТЕРМЕНИРОВАННЫХ.

Вот и думайте... О "экстраполяции"...

На двух..

Скажу лишь - "Когда вы говорите "у меня нет на это времени", на самом деле вы имеете ввиду, что вам это не важно. Нет времени на саморазвитие? На спорт, на семью?".

Ну и ещё - "Только никто не контролирует разработчика, он сам себя контролирует – «бьёт по рукам». Но это не мазохизм. Взамен разработчик получает кое-что важное – ощущение стабильности и уверенности в своём профессиональном завтрашнем дне. А как творец – он творит в два раза больше. Именно творит, а не «пашет». Пашет он как раз меньше.".