On 23-10-19 18:47, Andrew Eikum wrote:
On Mon, Oct 21, 2019 at 11:49:49PM +0200, Sven Baars wrote:
Signed-off-by: Sven Baars sven.wine@gmail.com
dlls/rpcrt4/rpc_server.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/dlls/rpcrt4/rpc_server.c b/dlls/rpcrt4/rpc_server.c index 12260b7298..b5110c879c 100644 --- a/dlls/rpcrt4/rpc_server.c +++ b/dlls/rpcrt4/rpc_server.c @@ -1558,12 +1558,16 @@ RPC_STATUS WINAPI RpcMgmtWaitServerListen( void ) WaitForSingleObject( event, INFINITE ); TRACE( "done waiting\n" );
- EnterCriticalSection(&listen_cs); /* wait for server threads to finish */ while(1) {
EnterCriticalSection(&listen_cs); if (listen_count){LeaveCriticalSection(&listen_cs); break;I agree there's a potential deadlock here (RpcMgmtWaitServerListen locks listen_cs and then server_cs, while RPCRT4_start_listen locks server_cs and then listen_cs), but this change unsynchronizes the listen_count check from the listen_done_event assignment below. That doesn't look right to me.
Andrew
Hi Andrew,
I thought that this was the reason that the "listen_done_event == event" statement is there in the first place. Note that before, during the while loop, listen_cs was already unlocked anyway, so in that case this issue also existed there.
But I indeed was also a bit confused because I suppose something like this could happen:
RPCRT4_stop_listen locks listen_cs RPCRT4_stop_listen checks if listen_done_event is NULL, it is not RPCRT4_stop_listen unlocks listen_cs RpcMgmtWaitServerListen locks listen_cs RpcMgmtWaitServerListen sets listen_done_event to NULL RpcMgmtWaitServerListen unlocks listen_cs RPCRT4_stop_listen locks listen_cs RPCRT4_stop_listen calls SetEvent with listen_done_event now being NULL RPCRT4_stop_listen unlocks listen_cs
and other similar things. But I'm not sure if these are actual issues that should be fixed. For the same reason I also wasn't really sure if I had to add that lock around listen_count.
Sven