[PATCH 4/4] ntdll: Avoid taking loader lock in LdrGetDllHandle().

Paul Gofman pgofman at codeweavers.com
Wed Nov 4 13:25:51 CST 2020


On 11/4/20 22:19, Alexandre Julliard wrote:
> Paul Gofman <pgofman at codeweavers.com> writes:
>
>> On 11/4/20 19:20, Alexandre Julliard wrote:
>>> Paul Gofman <pgofman at codeweavers.com> writes:
>>>
>>>> @@ -2267,12 +2267,17 @@ static NTSTATUS open_dll_file( UNICODE_STRING *nt_name, WINE_MODREF **pwm, void
>>>>      NTSTATUS status;
>>>>      HANDLE handle, mapping;
>>>>  
>>>> +    if (loaded_only)
>>>> +        lock_ldr_data();
>>>>      if ((*pwm = find_fullname_module( nt_name )))
>>>>      {
>>>>          NtUnmapViewOfSection( NtCurrentProcess(), *module );
>>>>          *module = NULL;
>>>> +        /* ldr_data_section to be unlocked by the caller. */
>>> Not releasing the lock in the same scope is ugly and fragile, please try
>>> to find a better way.
>>>
>> The straightforward alternative would be to factor out the dll finding
>> parts and use just them in LdrGetDllHandle().
>>
>> But looking at further removing loader lock from functions like
>> LdrAddRefDll and LdrUnloadDll it seems that it will be necessary to
>> introduce a per-module lock (presumably with SRW lock using Lock field
>> in struct _LDR_DATA_TABLE_ENTRY). And probably keeping locks / unlocks
>> contained in the same functions for that one will be much harder. Will
>> it work if I introduce this per-module lock before this patch and use a
>> concept of getting and putting module reference? That is, lock / unlock
>> of ldr_data will always happen in the same function, but if the module
>> reference is returned it will always have the lock on it and the caller
>> responsibility will be to unlock the module reference (pretty much
>> similar to how it is now done in wineserver WRT to getting and putting
>> object references)?
> That sounds better, yes.
>
Then maybe it is better to hold the whole series for now, as it would
likely make sense to change the first patch to lock / unlock
ldr_data_section in get_modref() instead of providing that lock in many
places outside.




More information about the wine-devel mailing list