From 97e397e768305ef78ef872f5f7aa6bb234013436 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sat, 28 Jan 2023 13:33:35 +0100 Subject: [PATCH] [server,shadow] simplify resource cleanup --- server/shadow/shadow.c | 43 ++++++++-------- server/shadow/shadow_client.c | 95 ++++++++++++++++------------------- server/shadow/shadow_server.c | 42 +++++----------- 3 files changed, 75 insertions(+), 105 deletions(-) diff --git a/server/shadow/shadow.c b/server/shadow/shadow.c index 11a88de25..ee42ff523 100644 --- a/server/shadow/shadow.c +++ b/server/shadow/shadow.c @@ -34,9 +34,9 @@ int main(int argc, char** argv) { int status = 0; - DWORD dwExitCode; - rdpSettings* settings; - rdpShadowServer* server; + DWORD dwExitCode = 0; + rdpSettings* settings = NULL; + rdpShadowServer* server = NULL; COMMAND_LINE_ARGUMENT_A shadow_args[] = { { "log-filters", COMMAND_LINE_VALUE_REQUIRED, ":[,:[,...]]", NULL, NULL, -1, NULL, "Set logger filters, see wLog(7) for details" }, @@ -104,26 +104,28 @@ int main(int argc, char** argv) { status = -1; WLog_ERR(TAG, "Server new failed"); - goto fail_server_new; + goto fail; } settings = server->settings; - settings->NlaSecurity = TRUE; - settings->TlsSecurity = TRUE; - settings->RdpSecurity = TRUE; + if (!freerdp_settings_set_bool(settings, FreeRDP_NlaSecurity, TRUE) || + !freerdp_settings_set_bool(settings, FreeRDP_TlsSecurity, TRUE) || + !freerdp_settings_set_bool(settings, FreeRDP_RdpSecurity, TRUE)) + goto fail; /* By default allow all GFX modes. * This can be changed with command line flags [+|-]gfx-CODEC */ - freerdp_settings_set_uint32(settings, FreeRDP_ColorDepth, 32); - freerdp_settings_set_bool(settings, FreeRDP_NSCodec, TRUE); - freerdp_settings_set_bool(settings, FreeRDP_RemoteFxCodec, TRUE); - freerdp_settings_set_bool(settings, FreeRDP_GfxH264, TRUE); - freerdp_settings_set_bool(settings, FreeRDP_GfxAVC444, TRUE); - freerdp_settings_set_bool(settings, FreeRDP_GfxAVC444v2, TRUE); - freerdp_settings_set_bool(settings, FreeRDP_GfxProgressive, TRUE); - freerdp_settings_set_bool(settings, FreeRDP_GfxProgressiveV2, TRUE); + if (!freerdp_settings_set_uint32(settings, FreeRDP_ColorDepth, 32) || + !freerdp_settings_set_bool(settings, FreeRDP_NSCodec, TRUE) || + !freerdp_settings_set_bool(settings, FreeRDP_RemoteFxCodec, TRUE) || + !freerdp_settings_set_bool(settings, FreeRDP_GfxH264, TRUE) || + !freerdp_settings_set_bool(settings, FreeRDP_GfxAVC444, TRUE) || + !freerdp_settings_set_bool(settings, FreeRDP_GfxAVC444v2, TRUE) || + !freerdp_settings_set_bool(settings, FreeRDP_GfxProgressive, TRUE) || + !freerdp_settings_set_bool(settings, FreeRDP_GfxProgressiveV2, TRUE)) + goto fail; #ifdef WITH_SHADOW_X11 server->authentication = TRUE; @@ -134,19 +136,19 @@ int main(int argc, char** argv) if ((status = shadow_server_parse_command_line(server, argc, argv, shadow_args)) < 0) { shadow_server_command_line_status_print(server, argc, argv, status, shadow_args); - goto fail_parse_command_line; + goto fail; } if ((status = shadow_server_init(server)) < 0) { WLog_ERR(TAG, "Server initialization failed."); - goto fail_server_init; + goto fail; } if ((status = shadow_server_start(server)) < 0) { WLog_ERR(TAG, "Failed to start server."); - goto fail_server_start; + goto fail; } #ifdef _WIN32 @@ -167,11 +169,8 @@ int main(int argc, char** argv) else status = (int)dwExitCode; -fail_server_start: +fail: shadow_server_uninit(server); -fail_server_init: -fail_parse_command_line: shadow_server_free(server); -fail_server_new: return status; } diff --git a/server/shadow/shadow_client.c b/server/shadow/shadow_client.c index fa87d8fa9..8e42f2e15 100644 --- a/server/shadow/shadow_client.c +++ b/server/shadow/shadow_client.c @@ -152,6 +152,35 @@ static INLINE void shadow_client_free_queued_message(void* obj) } } +static void shadow_client_context_free(freerdp_peer* peer, rdpContext* context) +{ + rdpShadowClient* client = (rdpShadowClient*)context; + rdpShadowServer* server; + + WINPR_UNUSED(peer); + if (!client) + return; + + server = client->server; + if (server && server->clients) + ArrayList_Remove(server->clients, (void*)client); + + if (client->encoder) + { + shadow_encoder_free(client->encoder); + client->encoder = NULL; + } + + /* Clear queued messages and free resource */ + WINPR_ASSERT(client->MsgQueue); + MessageQueue_Clear(client->MsgQueue); + MessageQueue_Free(client->MsgQueue); + WTSCloseServer((HANDLE)client->vcm); + client->vcm = NULL; + region16_uninit(&(client->invalidRegion)); + DeleteCriticalSection(&(client->lock)); +} + static BOOL shadow_client_context_new(freerdp_peer* peer, rdpContext* context) { BOOL NSCodec; @@ -197,10 +226,10 @@ static BOOL shadow_client_context_new(freerdp_peer* peer, rdpContext* context) settings->CompressionLevel = PACKET_COMPR_TYPE_RDP6; if (!freerdp_settings_set_string(settings, FreeRDP_CertificateFile, server->CertificateFile)) - goto fail_cert_file; + goto fail; if (!freerdp_settings_set_string(settings, FreeRDP_PrivateKeyFile, server->PrivateKeyFile)) - goto fail_privkey_file; + goto fail; if (server->ipcSocket && (strncmp(bind_address, server->ipcSocket, strnlen(bind_address, sizeof(bind_address))) != 0)) @@ -214,70 +243,30 @@ static BOOL shadow_client_context_new(freerdp_peer* peer, rdpContext* context) client->mayInteract = server->mayInteract; if (!InitializeCriticalSectionAndSpinCount(&(client->lock), 4000)) - goto fail_client_lock; + goto fail; region16_init(&(client->invalidRegion)); - client->vcm = WTSOpenServerA((LPSTR)peer->context); + client->vcm = WTSOpenServerA(peer->context); if (!client->vcm || client->vcm == INVALID_HANDLE_VALUE) - goto fail_open_server; + goto fail; if (!(client->MsgQueue = MessageQueue_New(&cb))) - goto fail_message_queue; + goto fail; if (!(client->encoder = shadow_encoder_new(client))) - goto fail_encoder_new; + goto fail; - if (ArrayList_Append(server->clients, (void*)client)) - return TRUE; + if (!ArrayList_Append(server->clients, (void*)client)) + goto fail; - shadow_encoder_free(client->encoder); - client->encoder = NULL; -fail_encoder_new: - MessageQueue_Free(client->MsgQueue); - client->MsgQueue = NULL; -fail_message_queue: - WTSCloseServer((HANDLE)client->vcm); - client->vcm = NULL; -fail_open_server: - DeleteCriticalSection(&(client->lock)); -fail_client_lock: - freerdp_settings_set_string(settings, FreeRDP_PrivateKeyFile, NULL); -fail_privkey_file: - freerdp_settings_set_string(settings, FreeRDP_CertificateFile, NULL); -fail_cert_file: + return TRUE; + +fail: + shadow_client_context_free(peer, client); return FALSE; } -static void shadow_client_context_free(freerdp_peer* peer, rdpContext* context) -{ - rdpShadowClient* client = (rdpShadowClient*)context; - rdpShadowServer* server; - - WINPR_ASSERT(context); - WINPR_UNUSED(peer); - server = client->server; - WINPR_ASSERT(server); - - WINPR_ASSERT(server->clients); - ArrayList_Remove(server->clients, (void*)client); - - if (client->encoder) - { - shadow_encoder_free(client->encoder); - client->encoder = NULL; - } - - /* Clear queued messages and free resource */ - WINPR_ASSERT(client->MsgQueue); - MessageQueue_Clear(client->MsgQueue); - MessageQueue_Free(client->MsgQueue); - WTSCloseServer((HANDLE)client->vcm); - client->vcm = NULL; - region16_uninit(&(client->invalidRegion)); - DeleteCriticalSection(&(client->lock)); -} - static INLINE void shadow_client_mark_invalid(rdpShadowClient* client, UINT32 numRects, const RECTANGLE_16* rects) { diff --git a/server/shadow/shadow_server.c b/server/shadow/shadow_server.c index 63776b304..d89c88543 100644 --- a/server/shadow/shadow_server.c +++ b/server/shadow/shadow_server.c @@ -822,62 +822,44 @@ int shadow_server_init(rdpShadowServer* server) WTSRegisterWtsApiFunctionTable(FreeRDP_InitWtsApi()); if (!(server->clients = ArrayList_New(TRUE))) - goto fail_client_array; + goto fail; if (!(server->StopEvent = CreateEvent(NULL, TRUE, FALSE, NULL))) - goto fail_stop_event; + goto fail; if (!InitializeCriticalSectionAndSpinCount(&(server->lock), 4000)) - goto fail_server_lock; + goto fail; status = shadow_server_init_config_path(server); if (status < 0) - goto fail_config_path; + goto fail; status = shadow_server_init_certificate(server); if (status < 0) - goto fail_certificate; + goto fail; server->listener = freerdp_listener_new(); if (!server->listener) - goto fail_listener; + goto fail; server->listener->info = (void*)server; server->listener->PeerAccepted = shadow_client_accepted; server->subsystem = shadow_subsystem_new(); if (!server->subsystem) - goto fail_subsystem_new; + goto fail; status = shadow_subsystem_init(server->subsystem, server); + if (status < 0) + goto fail; - if (status >= 0) - return status; + return status; - shadow_subsystem_free(server->subsystem); -fail_subsystem_new: - freerdp_listener_free(server->listener); - server->listener = NULL; -fail_listener: - free(server->CertificateFile); - server->CertificateFile = NULL; - free(server->PrivateKeyFile); - server->PrivateKeyFile = NULL; -fail_certificate: - free(server->ConfigPath); - server->ConfigPath = NULL; -fail_config_path: - DeleteCriticalSection(&(server->lock)); -fail_server_lock: - CloseHandle(server->StopEvent); - server->StopEvent = NULL; -fail_stop_event: - ArrayList_Free(server->clients); - server->clients = NULL; -fail_client_array: +fail: + shadow_server_uninit(server); WLog_ERR(TAG, "Failed to initialize shadow server"); return -1; }