From 02035b5a08762d935443d5ca49018f1a4c351a16 Mon Sep 17 00:00:00 2001 From: Zephyron Date: Thu, 30 Jan 2025 15:58:57 +1000 Subject: [PATCH] android: Fix multiplayer implementation issues - Add error handling for network initialization in JNI_OnLoad - Add input validation and timeout handling for room creation - Improve cleanup when leaving rooms to prevent memory leaks - Add error handling and logging for chat message processing - Improve thread safety for UI updates in message handling The main fixes include: 1. Network initialization now logs errors instead of failing silently 2. Room creation validates input parameters and has proper timeout handling 3. Room cleanup is now done in the correct order (clear chat, leave, destroy) 4. Chat message processing now has proper error handling and logging 5. UI updates are properly handled on the main thread These changes improve stability and error handling throughout the multiplayer implementation. --- .../citron/citron_emu/utils/NetPlayManager.kt | 41 +++++++++++-------- src/android/app/src/main/jni/multiplayer.cpp | 40 ++++++++++-------- src/common/android/id_cache.cpp | 8 +++- 3 files changed, 52 insertions(+), 37 deletions(-) diff --git a/src/android/app/src/main/java/org/citron/citron_emu/utils/NetPlayManager.kt b/src/android/app/src/main/java/org/citron/citron_emu/utils/NetPlayManager.kt index 1437e438b..1fdd9acfe 100644 --- a/src/android/app/src/main/java/org/citron/citron_emu/utils/NetPlayManager.kt +++ b/src/android/app/src/main/java/org/citron/citron_emu/utils/NetPlayManager.kt @@ -1,6 +1,6 @@ -// Copyright 2024 Mandarine Project -// Licensed under GPLv2 or any later version -// Refer to the license.txt file included. +// SPDX-FileCopyrightText: Copyright 2024 Mandarine Project +// SPDX-FileCopyrightText: Copyright 2025 citron Emulator Project +// SPDX-License-Identifier: GPL-2.0-or-later package org.citron.citron_emu.utils @@ -95,15 +95,21 @@ object NetPlayManager { when (type) { NetPlayStatus.CHAT_MESSAGE -> { - val parts = msg.split(":", limit = 2) - if (parts.size == 2) { - val nickname = parts[0].trim() - val chatMessage = parts[1].trim() - addChatMessage(ChatMessage( - nickname = nickname, - username = "", - message = chatMessage - )) + try { + val parts = msg.split(":", limit = 2) + if (parts.size == 2) { + val nickname = parts[0].trim() + val chatMessage = parts[1].trim() + addChatMessage(ChatMessage( + nickname = nickname, + username = "", + message = chatMessage + )) + } else { + Log.e("NetPlayManager", "Invalid chat message format: $msg") + } + } catch (e: Exception) { + Log.e("NetPlayManager", "Error processing chat message", e) } } NetPlayStatus.MEMBER_JOIN, @@ -118,13 +124,12 @@ object NetPlayManager { } } - - Handler(Looper.getMainLooper()).post { - if (!isChatOpen) { - Toast.makeText(context, message, Toast.LENGTH_SHORT).show() - } + // Show toast on UI thread if chat isn't open + Handler(Looper.getMainLooper()).post { + if (!isChatOpen) { + Toast.makeText(context, message, Toast.LENGTH_SHORT).show() } - + } messageListener?.invoke(type, msg) adapterRefreshListener?.invoke(type, msg) diff --git a/src/android/app/src/main/jni/multiplayer.cpp b/src/android/app/src/main/jni/multiplayer.cpp index 1b41ca1f1..84c1c27ac 100644 --- a/src/android/app/src/main/jni/multiplayer.cpp +++ b/src/android/app/src/main/jni/multiplayer.cpp @@ -1,6 +1,7 @@ -// Copyright 2025 Citra Project / Mandarine Project -// Licensed under GPLv2 or any later version -// Refer to the license.txt file included. +// SPDX-FileCopyrightText: Copyright 2025 citra Emulator Project +// SPDX-FileCopyrightText: Copyright 2025 mandarine Emulator Project +// SPDX-FileCopyrightText: Copyright 2025 citron Emulator Project +// SPDX-License-Identifier: GPL-2.0-or-later #include "multiplayer.h" #include "core/core.h" #include "network/network.h" @@ -139,6 +140,11 @@ const AnnounceMultiplayerRoom::GameInfo game{ NetPlayStatus NetPlayCreateRoom(const std::string& ipaddress, int port, const std::string& username, const std::string& password, const std::string& room_name, int max_players) { + // Validate input parameters + if (ipaddress.empty() || port <= 0 || port > 65535 || username.empty() || + room_name.length() < 3 || room_name.length() > 20 || max_players < 2 || max_players > 16) { + return NetPlayStatus::CREATE_ROOM_ERROR; + } auto member = Network::RoomNetwork().GetRoomMember().lock(); if (!member) { @@ -163,13 +169,9 @@ NetPlayStatus NetPlayCreateRoom(const std::string& ipaddress, int port, const st return NetPlayStatus::CREATE_ROOM_ERROR; } - // Failsafe timer to avoid joining before creation - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - - member->Join(username, ipaddress.c_str(), port, 0, Network::NoPreferredIP, password); - - // Failsafe timer to avoid joining before creation - for (int i = 0; i < 5; i++) { + // Add timeout for room creation + const int MAX_CREATION_ATTEMPTS = 10; + for (int i = 0; i < MAX_CREATION_ATTEMPTS; i++) { std::this_thread::sleep_for(std::chrono::milliseconds(100)); if (member->GetState() == Network::RoomMember::State::Joined || member->GetState() == Network::RoomMember::State::Moderator) { @@ -177,7 +179,7 @@ NetPlayStatus NetPlayCreateRoom(const std::string& ipaddress, int port, const st } } - // If join failed while room is created, clean up the room + // Cleanup on failure room->Destroy(); return NetPlayStatus::CREATE_ROOM_ERROR; } @@ -289,14 +291,18 @@ bool NetPlayIsHostedRoom() { void NetPlayLeaveRoom() { if (auto room = Network::RoomNetwork().GetRoom().lock()) { - // if you are in a room, leave it - if (auto member = Network::RoomNetwork().GetRoomMember().lock()) { - member->Leave(); - } - + // Clear chat before leaving to prevent memory leaks ClearChat(); - // if you are hosting a room, also stop hosting + // Leave room first if connected + if (auto member = Network::RoomNetwork().GetRoomMember().lock()) { + member->Leave(); + + // Wait for leave to complete + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } + + // Then destroy room if hosting if (room->GetState() == Network::Room::State::Open) { room->Destroy(); } diff --git a/src/common/android/id_cache.cpp b/src/common/android/id_cache.cpp index cffaef637..935c5b9df 100644 --- a/src/common/android/id_cache.cpp +++ b/src/common/android/id_cache.cpp @@ -1,4 +1,5 @@ // SPDX-FileCopyrightText: Copyright 2023 yuzu Emulator Project +// SPDX-FileCopyrightText: Copyright 2025 citron Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later #include @@ -569,8 +570,11 @@ jint JNI_OnLoad(JavaVM* vm, void* reserved) { // Initialize applets Common::Android::SoftwareKeyboard::InitJNI(env); - // Init network for multiplayer - NetworkInit(); + // Initialize network for multiplayer - add error handling + if (!NetworkInit()) { + LOG_ERROR(Network, "Failed to initialize network for multiplayer"); + // Don't fail JNI_OnLoad completely, just log the error + } return JNI_VERSION; }