Очередной баг или так и задумано

← →
oxffff © (2007-07-29 23:26) [0]

Существует непустая глобальная переменная

b:Iunknown;

Представьте два потока, которые одновременно пытаются выполнить следующий код

b:=nil;

Вот непосредственно код соответствия на Asm

function _IntfClear(var Dest: IInterface): Pointer;
asm
MOV EDX,[EAX]
TEST EDX,EDX
JE @@1
MOV DWORD PTR [EAX],0
PUSH EAX
PUSH EDX
MOV EAX,[EDX]
CALL DWORD PTR [EAX] + VMTOFFSET IInterface._Release
POP EAX
@@1:
end;

Я его слегка упрощу, чтобы более легче понять его. Уберем проверку на nil и сохранение eax;

function _IntfClear(var Dest: IInterface): Pointer;
asm
MOV EDX,[EAX]
MOV DWORD PTR [EAX],0
PUSH EDX
MOV EAX,[EDX]
CALL DWORD PTR [EAX] + VMTOFFSET IInterface._Release
end;

Теперь вернемся к нашем двум потокам.
Вообразите ситуацию, когда первый поток еще не выполнил _IntfClear.
Происходит переключение на второй поток, который выполняет _IntfClear и выделяет память по потому же адресу и обнуляет ее.

Вот сериализованный поток иструкций с комментариями

procedure TForm1.Button1Click(Sender: TObject);
var a:TinterfacedObject;
begin
a:=TinterfacedObject.Create;
b:=a;
asm
push ebx; //Сохраняем ebx.

//Первый поток
mov eax,b;

push eax; //Сохранение контекста потока и переключение на второй поток.

//Второй поток

mov eax,b; //
mov b,0;
push eax;
push eax;
mov eax,[eax];
call [eax+$08];

pop eax; // Была выделена память на том же самом месте, где и объект и была очищена.
mov [eax],0;

//обратное переключение на первый поток
pop eax; //Восстановление контекста потока .

mov b,0;
push eax;
mov eax,[eax];
call [eax+$08]; // Здесь мы получаем AV.

pop ebx;//Восстанавливаем ebx
end;
b:=nil;
end;

Получение AV вызвано тем, что текущая реализация _IntfClear расчитана на один поток.

Вопрос такой

это баг. Или As designed?

← →
oxffff © (2007-07-29 23:28) [1]

Такая ситуация может возникнуть и для _IntfCopy

← →
jack128_ (2007-07-29 23:35) [2]

А с чего ты взял, что дельфя такие вещи должна синхронизировать??

← →
oxffff © (2007-07-29 23:40) [3]

> jack128_ (29.07.07 23:35) [2]
> А с чего ты взял, что дельфя такие вещи должна синхронизировать?
> ?

В смысле?

Я привел пример, который демонстрирует потенциальную возможность AV при одновременной очистке интерфейсной переменной из двух разных потоков.

Собственно _IntfCopy и _IntfClear можно поправить, чтобы подобной ситуации не возникло.

← →
oxffff © (2007-07-29 23:41) [4]

> jack128_ (29.07.07 23:35) [2]
> А с чего ты взял, что дельфя такие вещи должна синхронизировать?
> ?

А взял я это с того, что interface это тип Delphi. И код финализации должен быть потокозащищенным.

:)

← →
oxffff © (2007-07-29 23:50) [5]

Для строк тоже самое. См. _LStrClr

← →
Lacmus © (2007-07-29 23:50) [6]

Почему бы не продолжить :)

b: string;
b: array of ...;

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

← →
oxffff © (2007-07-29 23:58) [7]

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

Ну не только глобальными. А с разделяемыми.

Вы конечно скажите, так это и есть защищаемый ресурс.

Только, если внимательно рассмотреть.
Операция присваивания должна быть атомарна.
И защищать ее не следуют.

Собственно спор здесь не уместен.

Я думаю вы не откажитесь, от того чтобы операция присваивания была атомарна?

← →
Lacmus © (2007-07-30 00:06) [8]

>Я думаю вы не откажитесь, от того чтобы операция присваивания была атомарна?

Речь идет об этом ?

MOV DWORD PTR [EAX],0

← →
jack128_ (2007-07-30 00:32) [9]

> Операция присваивания должна быть атомарна.

Ну..В свете того, что неявное приведение типов у нас теперь может быть перегружено, это более чем спорное утверждение.

← →
Anatoly Podgoretsky © (2007-07-30 00:34) [10]

Я бы ни чему, кроме ассемблера не доверял.

← →
oxffff © (2007-07-30 08:51) [11]

> Речь идет об этом ?
>
> MOV DWORD PTR [EAX],0

Вот об этом

MOV EDX,[EAX]
TEST EDX,EDX
JE @@1
MOV DWORD PTR [EAX],0

Заменить либо lock cmpxсhg, либо просто xchg

← →
Lacmus © (2007-07-30 11:08) [12]

>oxffff © (30.07.07 08:51) [11]

Предлагаете заменить все присвоения в программе или только это ?

← →
oxffff © (2007-07-30 11:23) [13]

> Lacmus © (30.07.07 11:08) [12]
> >oxffff © (30.07.07 08:51) [11]
>
> Предлагаете заменить все присвоения в программе или только
> это ?

Предлагаю сделать threadSafe assignment

http://qc.codegear.com/wc/qcmain.aspx?d=49745

Все Ref Counting типы (string, DynArray, Interface).

Т.е модифицировать по аналогии _IntfClear, _IntfCopy и др.

← →
Lacmus © (2007-07-30 11:30) [14]

>oxffff © (30.07.07 11:23) [13]

Если переменная объявлена как

var
gPtr: Pointer = nil;

то с ее присвоением делать ничего не надо, так ?

← →
oxffff © (2007-07-30 11:38) [15]

> Lacmus © (30.07.07 11:30) [14]
> >oxffff © (30.07.07 11:23) [13]
>
> Если переменная объявлена как
>
> var
> gPtr: Pointer = nil;
>
> то с ее присвоением делать ничего не надо, так ?

Именно так.

Ближе к делу. :)

← →
Lacmus © (2007-07-30 11:56) [16]

т.е. код

if gPtr = nil then gPtr := TMyClass.Create(nil)

будет threadSafe ?

← →
oxffff © (2007-07-30 12:19) [17]

> Lacmus © (30.07.07 11:56) [16]
> т.е. код
>
>
> if gPtr = nil then
> gPtr := TMyClass.Create(nil)
>
>
> будет threadSafe ?

Нет. Не будет.

Т.е. для указателя выполняется код

mov [ebp-$..],eax.

С предыдущим значением нет манипуляций.

Я предлагаю сделать безопасное присваивание для string, DynArray, Interface.

А для ref counting типов выполняется еще и Counting.

Разница лишь в том, что операция изъятия и изменения значения должна быть атомарна.

Что не приведет к повторному counting для другого потока.

Хотя я тут подумал и нашел проблему посерьезней. :)

PUSH EAX
MOV EAX,[EDX]
CALL DWORD PTR [EAX] + VMTOFFSET Method

Это никак не защитишь.

Вопрос я думаю закрыт. Мое предложение некорректно.

:)

так IInterface, скорее всего и не задумывался как потокобезопасный. Это основа для поддержки СОМ, а там просто так ссылку на интерфейс в другой поток не передашь.

> PUSH EDX
> MOV EAX,[EDX]
> CALL DWORD PTR [EAX] + VMTOFFSET Method

> umbra © (30.07.07 12:30) [18]
> так IInterface, скорее всего и не задумывался как потокобезопасный.
> Это основа для поддержки СОМ, а там просто так ссылку на
> интерфейс в другой поток не передашь.

Да я тут хотел выдумать threadsafe работу c ref counting типами.
Только работа с ними к сожалению (точнее к счастью) не ограничивается
исключительно операциями присваивания. Об этом я что то не подумал.

:)

Хотя можно усовершенствовать саму операцию присваивания, таким способом, чтобы она была бы threadSafe.

А вот другие операции требуют guard section.

;)

так может эти методы потокобезопасны :) DВ случае с IInterface, например, в TInterfacedObject._Release для уменьшения счетчика ссылок используется потокобезопасная InterlockedDecrement

а в Вашем примере вы вообще не имеете дела с интерфейсом, Вы работаете с объектом.

> umbra © (30.07.07 12:56) [21]
> так может эти методы потокобезопасны :) DВ случае с IInterface,
> например, в TInterfacedObject._Release для уменьшения счетчика
> ссылок используется потокобезопасная InterlockedDecrement

Вы меня не поняли.

Я имею ввиду сам interface method invoking и сама работа метода в случае shared variable небезопасен , а именно код вызова

PUSH EDX
MOV EAX,[EDX]
CALL DWORD PTR [EAX] + VMTOFFSET Method

и работа метода _Release. и указанный вами InterlockedDecrement небезопасен. По причине, того что другой поток может "отпустить" интерфейс в shared variable и разрушить объект.
Таким образом другой поток разрушил объект. а Первый еще работает с ним через тот же shared variable. А InterlockedDecrement отработает по убитому объекту. Последствия могут быть разные.

> umbra © (30.07.07 13:07) [22]
> а в Вашем примере вы вообще не имеете дела с интерфейсом,
> Вы работаете с объектом.

Так по порядку. См [0].
Global

b:Iunknown;

Local

a:TinterfacedObject;

a:=TinterfacedObject.Create;
b:=a;

...
mov eax,b;
mov b,0;
push eax;
mov eax,[eax];
call [eax+$08];

...

Так что все же это интерфейс.

> umbra © (30.07.07 12:56) [21]
> так может эти методы потокобезопасны :) DВ случае с IInterface,
> например, в TInterfacedObject._Release для уменьшения счетчика
> ссылок используется потокобезопасная InterlockedDecrement

И еще до самого вызова может дело не дойти.

PUSH EDX
вмешивается второй поток и "отпускает" интерфейс shared variable. Далее выделяется память по тому же месту. И обнуляется.

переключается обратно
MOV EAX,[EDX] //теперь EAX=0
CALL DWORD PTR [EAX+VMTOFFSET Method] //Здесь AV

Очередной баг или так и задумано Найти похожие ветки