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

[Alexandre Julliard]

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.

-- 
Alexandre Julliard
julliard at winehq.org