Мне прислали задачку как раз очень в тему моей серии статей про плагины.
Предположим, что у нас есть такой класс:
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 во время выгрузки плагинов. Не могли бы вы указать на неё?
Дополнительно: как бы вы исправили её и как бы диагностировали?
Ответ.
Предполагаю, что проблема может быть, если есть зависимости между плагинами. Тогда сначала нужно финализировать все плагины, а только потом в цикле освобождать указатели и выгружать пакеты.
ОтветитьУдалитьЭто связано с
ОтветитьУдалитьPlugin: IPlugin;
т.к. происходит двойное удаление объекта, т.к при присвоении к nil - объект удаляется автоматически, это правило COM, второй раз принудительно его удалять не надобно
Ну и еще лучше конечно
ОтветитьУдалитьслелать так, для надежности
for i := Count - 1 downto 0 do
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));
У меня мало опыта работы с интерфейсами, но попробую поразмышлять и описать, что меня смущает в коде..
ОтветитьУдалить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. Есть такое негласное правило, что освобождать объекты надо в обратном от создания порядке. Это то, о чём написал Дмитрий.
В данном конкретном случае это не обязательно, но как-то бросается в глаза и только отвлекает (хотя, дело привычки).
100% проблема в неявной ссылке на интерфейс, которая живет до завершения метода. Сам на эти грабли наступал.
ОтветитьУдалитьНиколай Зверев прав, проблема в первом пункте.
ОтветитьУдалитьКстати, даже если учесть эту проблему, на тестовом примере есть риск наступить на ещё одни грабли (как было со мной, решая эту задачку).
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;
функция 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;
Прошу прощения, что названия классов и интерфейсов имеют несколько иные написание от предложенных в задаче ))
ОтветитьУдалитьЕдинственное могу сказать "Спасибо" GunSmoker'у за его публикации, побудившие поглубже разобраться в плагинах и интерфейсах - тема очень интересная и занимательная.. заставила наш отдел поломать голову в поисках оптимального решения для нашей задачи - в результате получился гибрид метода, изложенного GunSmoker'ом в серии статей о плагинах, идеологии, которую требовало начальство и имеющегося собственного опыта разработок..
Ссылка в конце задачи ("ответ") ведет не на ответ на этот вопрос, а на следующий вопрос (№14).
ОтветитьУдалитьСпасибо, исправил.
ОтветитьУдалить