9 марта 2012 г.

Задачка №13

В чём проблема в этом коде?

Мне прислали задачку как раз очень в тему моей серии статей про плагины.

Предположим, что у нас есть такой класс:
TPlugin = record
  Handle: HMODULE;
  Plugin: IPlugin;
end;

TPluginManager = class(TInterfacedObject, IPluginManager)
private
  FItems: array of TPlugin;
  ...
protected
  function GetItem(const AIndex: Integer): IPlugin;
  function GetCount: Integer;
  ...
public
  ...
  property Items[const AIndex: Integer]: IPlugin read GetItem; default;
  property Count: Integer read GetCount; 
  ...
  procedure UnloadAll;
  ...
end;
Как видите, это немного иначе сделанный менеджер плагинов, который хранит список плагинов в виде массива записей TPlugin. В записи хранится описатель библиотеки плагина и интерфейс плагина.

И предположим, что метод UnloadAll выглядит так:
procedure TPluginManager.UnloadAll;
var
  i: Integer;
begin
  // Вызываем Done, чтобы плагины корректно финализировались
  for i := 0 to Count - 1 do
    Items[i].Done;

  // Чистим записи
  for i := 0 to Count - 1 do
  begin
    FItems[i].Plugin := nil;
    FreeLibrary(FItems[i].Handle);
  end;

  Finalize(FItems);
end;
Сначала мы вызываем функцию финализации плагинов Done (например, чтобы плагины отпустили бы ссылки), а затем выгружаем плагины. При этом мы сначала отпускаем ссылку на плагин, а затем выгружаем библиотеку. Это сделано, чтобы плагин уничтожился бы до выгрузки своей библиотеки, а не после, избегая, таким образом, вылета программы при попытке доступа к уже выгруженному плагину.

Тем не менее, в этом коде есть ошибка, которая приводит к Access Violation во время выгрузки плагинов. Не могли бы вы указать на неё?

Дополнительно: как бы вы исправили её и как бы диагностировали?

Ответ.

11 комментариев :

  1. Предполагаю, что проблема может быть, если есть зависимости между плагинами. Тогда сначала нужно финализировать все плагины, а только потом в цикле освобождать указатели и выгружать пакеты.

    ОтветитьУдалить
  2. Это связано с
    Plugin: IPlugin;
    т.к. происходит двойное удаление объекта, т.к при присвоении к nil - объект удаляется автоматически, это правило COM, второй раз принудительно его удалять не надобно

    ОтветитьУдалить
  3. Ну и еще лучше конечно
    слелать так, для надежности
    for i := Count - 1 downto 0 do

    ОтветитьУдалить
  4. Человек_Борща9 марта 2012 г., 21:06

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

    Я бы сделал так, раз уж у нас Record's:

    // Чистим записи
    for i := 0 to Count - 1 do
    begin
    FItems[i].Plugin := nil;
    FreeLibrary(FItems[i].Handle);
    //Очтщаем ячейку
    ZeroMemory(@fItems[i],SizeOf(TPlugin)); //Может будет лишним
    end;

    //Очищаем массив
    ZeroMemory(@fItems,SizeOf(TPlugin));

    ОтветитьУдалить
  5. У меня мало опыта работы с интерфейсами, но попробую поразмышлять и описать, что меня смущает в коде..

    1. Строка 7: Items[i].Done.
    Обращение к свойству Items[i] приведёт к вызову метода GetItem, а это в свою очередь приведёт к созданию локальной переменной-ссылки на объект ("под капотом языка") и к наращиванию ссылок на объект (addref). Затем происходит вызов Done, а затем область видимости локальной переменной вроде бы и заканчивается и должно произойти уменьшение ссылок (release). Но вполне возможно, что область видимости этой локальной переменной на самом деле заканчивается не сразу после вызова метода Done, а лишь при выходе из метода UnloadAll. Т.е. вызов release вызовется при следующей итерации цикла при замене значения ссылки на предыдущий плагин (i-1) на текущий (i). А после цикла у нас останется неявная ссылка на последний плагин, которая и будет его "держать". А значит последний плагин и вызовет AV (ведь его DLL уже освободили, а память ещё нет).

    Проверить это можно, реализовав свои методы AddRef и Release, на примере последней статьи серии.
    Ну и разрешить этот вопрос можно заменив
    Items[i].Done
    на
    FItems[i].Plugin.Done

    2. Не плохо бы проверять кол-во ссылок на плагин перед явным вызовом FreeLibrary, ведь если вдруг ещё кто-то "держит ссылку" - то это ещё одна проблема.

    3. Я не вижу смысла в явном вызове Finalize(FItems), по моему достаточно будет сделать SetLength(FItems, 0)? В любом случае, деструктор менеджера сделает Finalize для всех своих полей. Да и потом, чего там финализировать, ведь ссылки на объекты уже обнулены.

    4. Есть такое негласное правило, что освобождать объекты надо в обратном от создания порядке. Это то, о чём написал Дмитрий.
    В данном конкретном случае это не обязательно, но как-то бросается в глаза и только отвлекает (хотя, дело привычки).

    ОтветитьУдалить
  6. 100% проблема в неявной ссылке на интерфейс, которая живет до завершения метода. Сам на эти грабли наступал.

    ОтветитьУдалить
  7. Николай Зверев прав, проблема в первом пункте.

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


    var
    PM: TPluginManager;
    Handle: HMODULE;
    GetPluginProc: TGetPluginProc;
    begin
    PM := TPluginManager.Create;

    Handle := LoadLibrary('Plugin.dll');

    GetPluginProc := GetProcAddress(Handle, 'GetPluginProc');

    PM.AddItem(Handle, GetPluginProc);

    PM.UnloadAll;
    //тут тоже будет AV :)
    end;

    ...

    function GetPluginProc: IPlugin;
    begin
    Result := TPlugin.Create;
    end;

    ОтветитьУдалить
  8. функция GetItem в этом примере реализована так:
    function TPluginManager.GetItem(const AIndex: Integer): IMG_Module;
    begin
    Result:=fItems[AIndex];
    end;

    Заполнение массива fItems идёт во время загрузки плагинов.. Никаких пустых элементов просто нет физически..

    В окончательном виде процедура UnloadAll выглядит так:

    procedure TMediator.UnloadAll;
    begin
    Finalize(fItems);
    FreeAndNil(fProviders);
    end;

    ОтветитьУдалить
  9. Прошу прощения, что названия классов и интерфейсов имеют несколько иные написание от предложенных в задаче ))

    Единственное могу сказать "Спасибо" GunSmoker'у за его публикации, побудившие поглубже разобраться в плагинах и интерфейсах - тема очень интересная и занимательная.. заставила наш отдел поломать голову в поисках оптимального решения для нашей задачи - в результате получился гибрид метода, изложенного GunSmoker'ом в серии статей о плагинах, идеологии, которую требовало начальство и имеющегося собственного опыта разработок..

    ОтветитьУдалить
  10. Ссылка в конце задачи ("ответ") ведет не на ответ на этот вопрос, а на следующий вопрос (№14).

    ОтветитьУдалить

Можно использовать некоторые HTML-теги, например:

<b>Жирный</b>
<i>Курсив</i>
<a href="http://www.example.com/">Ссылка</a>

Вам необязательно регистрироваться для комментирования - для этого просто выберите из списка "Анонимный" (для анонимного комментария) или "Имя/URL" (для указания вашего имени и (опционально) ссылки на сайт). Все прочие варианты потребуют от вас входа в вашу учётку (поддерживается OpenID).

Пожалуйста, по возможности используйте "Имя/URL" вместо "Анонимный". URL можно просто не указывать.

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