Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

NetKDRequestDevice: Fix sporadic crashes during emulation shutdown #12463

Merged
merged 2 commits into from
Dec 27, 2023
Merged

NetKDRequestDevice: Fix sporadic crashes during emulation shutdown #12463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd1f5f9
NetKDRequestDevice: Fix use-after-free crash
Dentomologist Dec 21, 2023
ecf4f1b
NetKDRequestDevice: Fix nullptr dereference crash
Dentomologist Dec 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions Source/Core/Core/IOS/IOS.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,8 +577,11 @@ void EmulationKernel::AddStaticDevices()
}
if (HasFeature(features, Feature::KD))
{
AddDevice(std::make_unique<NetKDRequestDevice>(*this, "/dev/net/kd/request"));
AddDevice(std::make_unique<NetKDTimeDevice>(*this, "/dev/net/kd/time"));
constexpr auto time_device_name = "/dev/net/kd/time";
AddDevice(std::make_unique<NetKDTimeDevice>(*this, time_device_name));
const auto time_device =
std::static_pointer_cast<NetKDTimeDevice>(GetDeviceByName(time_device_name));
AddDevice(std::make_unique<NetKDRequestDevice>(*this, "/dev/net/kd/request", time_device));
}
if (HasFeature(features, Feature::NCD))
{
Expand Down
19 changes: 8 additions & 11 deletions Source/Core/Core/IOS/Network/KD/NetKDRequest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,10 @@ s32 NWC24MakeUserID(u64* nwc24_id, u32 hollywood_id, u16 id_ctr, HardwareModel h
}
} // Anonymous namespace

NetKDRequestDevice::NetKDRequestDevice(EmulationKernel& ios, const std::string& device_name)
NetKDRequestDevice::NetKDRequestDevice(EmulationKernel& ios, const std::string& device_name,
const std::shared_ptr<NetKDTimeDevice>& time_device)
: EmulationDevice(ios, device_name), m_config{ios.GetFS()}, m_dl_list{ios.GetFS()},
m_send_list{ios.GetFS()}, m_friend_list{ios.GetFS()}
m_send_list{ios.GetFS()}, m_friend_list{ios.GetFS()}, m_time_device{time_device}
{
// Enable all NWC24 permissions
m_scheduler_buffer[1] = Common::swap32(-1);
Expand Down Expand Up @@ -190,6 +191,8 @@ NetKDRequestDevice::~NetKDRequestDevice()
}

m_scheduler_timer_thread.join();
m_scheduler_work_queue.Shutdown();
m_work_queue.Shutdown();
Comment on lines +194 to +195
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling Shutdown(true) would cancel the remaining items and be a bit faster, but I'm not familiar enough with NetKD to know if that would cause any problems.

}

void NetKDRequestDevice::Update()
Expand Down Expand Up @@ -441,9 +444,7 @@ NWC24::ErrorCode NetKDRequestDevice::DetermineDownloadTask(u16* entry_index,
// As the scheduler does not tell us which entry to download, we must determine that.
// A correct entry is one that hasn't been downloaded the longest compared to other entries.
// We first need current UTC.
const auto time_device =
std::static_pointer_cast<NetKDTimeDevice>(GetIOS()->GetDeviceByName("/dev/net/kd/time"));
const u64 current_utc = time_device->GetAdjustedUTC();
const u64 current_utc = m_time_device->GetAdjustedUTC();
u64 lowest_timestamp = std::numeric_limits<u64>::max();

for (u16 i = 0; i < static_cast<u16>(NWC24::NWC24Dl::MAX_ENTRIES); i++)
Expand Down Expand Up @@ -493,9 +494,7 @@ NWC24::ErrorCode NetKDRequestDevice::DetermineSubtask(u16 entry_index,
if (m_dl_list.IsSubtaskDownloadDisabled(entry_index))
return NWC24::WC24_ERR_DISABLED;

const auto time_device =
std::static_pointer_cast<NetKDTimeDevice>(GetIOS()->GetDeviceByName("/dev/net/kd/time"));
const u64 current_utc = time_device->GetAdjustedUTC();
const u64 current_utc = m_time_device->GetAdjustedUTC();
for (u8 i = 0; i < 32; i++)
{
if (!m_dl_list.IsValidSubtask(entry_index, i))
Expand Down Expand Up @@ -645,9 +644,7 @@ NWC24::ErrorCode NetKDRequestDevice::KDDownload(const u16 entry_index,
{
bool success = false;
Common::ScopeGuard state_guard([&] {
const auto time_device =
std::static_pointer_cast<NetKDTimeDevice>(GetIOS()->GetDeviceByName("/dev/net/kd/time"));
const u64 current_utc = time_device->GetAdjustedUTC();
const u64 current_utc = m_time_device->GetAdjustedUTC();
if (success)
{
// Set the next download time to the dl_margin
Expand Down
6 changes: 5 additions & 1 deletion Source/Core/Core/IOS/Network/KD/NetKDRequest.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#pragma once

#include <array>
#include <memory>
#include <queue>
#include <string>

Expand All @@ -17,6 +18,7 @@
#include "Core/IOS/Network/KD/Mail/WC24Send.h"
#include "Core/IOS/Network/KD/NWC24Config.h"
#include "Core/IOS/Network/KD/NWC24DL.h"
#include "Core/IOS/Network/KD/NetKDTime.h"
Comment on lines 19 to +21
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems clang-format doesn't sort it alphabetically correctly (or rather its sorting is not case insensitive, probably ASCII based since lowercase is after uppercase). Feel free to ignore this comment since there is not much you can do as it isn't related to this PR.


namespace IOS::HLE
{
Expand All @@ -26,7 +28,8 @@ namespace IOS::HLE
class NetKDRequestDevice : public EmulationDevice
{
public:
NetKDRequestDevice(EmulationKernel& ios, const std::string& device_name);
NetKDRequestDevice(EmulationKernel& ios, const std::string& device_name,
const std::shared_ptr<NetKDTimeDevice>& time_device);
IPCReply HandleNWC24DownloadNowEx(const IOCtlRequest& request);
NWC24::ErrorCode KDDownload(const u16 entry_index, const std::optional<u8> subtask_id);
IPCReply HandleNWC24CheckMailNow(const IOCtlRequest& request);
Expand Down Expand Up @@ -114,6 +117,7 @@ class NetKDRequestDevice : public EmulationDevice
std::queue<AsyncReply> m_async_replies;
u32 m_error_count = 0;
std::array<u32, 256> m_scheduler_buffer{};
std::shared_ptr<NetKDTimeDevice> m_time_device;
// TODO: Maybe move away from Common::HttpRequest?
Common::HttpRequest m_http{std::chrono::minutes{1}};
u32 m_download_span = 2;
Expand Down