Race condition when stopping while emu is failing

Emulator::stop now throws if an error occurred on the emu thread.
Fixes MINIDUMP-5B
This commit is contained in:
Flyinghead 2023-02-26 22:01:32 +01:00
parent e505ab6019
commit 30501aebb1
11 changed files with 160 additions and 86 deletions

View File

@ -76,6 +76,24 @@ const std::array<Sh4RegType, 59> Sh4RegList {
class DebugAgent
{
public:
struct Breakpoint {
enum Type
{
BP_TYPE_SOFTWARE_BREAK,
BP_TYPE_HARDWARE_BREAK,
BP_TYPE_WRITE_WATCHPOINT,
BP_TYPE_READ_WATCHPOINT,
BP_TYPE_ACCESS_WATCHPOINT,
BP_TYPE_COUNT
};
Breakpoint() = default;
Breakpoint(u16 type, u32 addr) : addr(addr), type(type) { }
u32 addr = 0;
u16 type = 0;
u16 savedOp = 0;
};
void doContinue(u32 pc = 1)
{
if (pc != 1)
@ -85,20 +103,20 @@ public:
void step()
{
bool restoreBreakpoint = removeMatchpoint(Breakpoint::Type::BP_TYPE_SOFTWARE_BREAK, Sh4cntx.pc, 2);
bool restoreBreakpoint = removeMatchpoint(Breakpoint::BP_TYPE_SOFTWARE_BREAK, Sh4cntx.pc, 2);
u32 savedPc = Sh4cntx.pc;
emu.step();
if (restoreBreakpoint)
insertMatchpoint(Breakpoint::Type::BP_TYPE_SOFTWARE_BREAK, savedPc, 2);
insertMatchpoint(Breakpoint::BP_TYPE_SOFTWARE_BREAK, savedPc, 2);
}
void stepRange(u32 from, u32 to)
{
bool restoreBreakpoint = removeMatchpoint(Breakpoint::Type::BP_TYPE_SOFTWARE_BREAK, Sh4cntx.pc, 2);
bool restoreBreakpoint = removeMatchpoint(Breakpoint::BP_TYPE_SOFTWARE_BREAK, Sh4cntx.pc, 2);
u32 savedPc = Sh4cntx.pc;
emu.stepRange(from, to);
if (restoreBreakpoint)
insertMatchpoint(Breakpoint::Type::BP_TYPE_SOFTWARE_BREAK, savedPc, 2);
insertMatchpoint(Breakpoint::BP_TYPE_SOFTWARE_BREAK, savedPc, 2);
}
int readAllRegs(u32 **regs)
@ -202,9 +220,9 @@ public:
}
}
}
bool insertMatchpoint(char type, u32 addr, u32 len)
bool insertMatchpoint(Breakpoint::Type type, u32 addr, u32 len)
{
if (type == 0 && len != 2) {
if (type == Breakpoint::BP_TYPE_SOFTWARE_BREAK && len != 2) {
WARN_LOG(COMMON, "insertMatchpoint: length != 2: %d", len);
return false;
}
@ -216,9 +234,9 @@ public:
WriteMem16_nommu(addr, 0xC308); // trapa #0x20
return true;
}
bool removeMatchpoint(char type, u32 addr, u32 len)
bool removeMatchpoint(Breakpoint::Type type, u32 addr, u32 len)
{
if (type == 0 && len != 2) {
if (type == Breakpoint::BP_TYPE_SOFTWARE_BREAK && len != 2) {
WARN_LOG(COMMON, "removeMatchpoint: length != 2: %d", len);
return false;
}
@ -327,23 +345,6 @@ public:
u32 exception = 0;
struct Breakpoint {
enum Type
{
BP_TYPE_SOFTWARE_BREAK,
BP_TYPE_HARDWARE_BREAK,
BP_TYPE_WRITE_WATCHPOINT,
BP_TYPE_READ_WATCHPOINT,
BP_TYPE_ACCESS_WATCHPOINT,
BP_TYPE_COUNT
};
Breakpoint() = default;
Breakpoint(u16 type, u32 addr) : addr(addr), type(type) { }
u32 addr = 0;
u16 type = 0;
u16 savedOp = 0;
};
std::map<u32, Breakpoint> breakpoints[Breakpoint::Type::BP_TYPE_COUNT];
std::vector<std::pair<u32, u32>> stack;
};

View File

@ -103,7 +103,7 @@ public:
if (config::GDBWaitForConnection)
{
DEBUG_LOG(COMMON, "Waiting for GDB connection...");
agent.interrupt();
agentInterrupt();
}
}
@ -198,18 +198,22 @@ private:
{
NOTICE_LOG(NETWORK, "gdb: client connection");
attached = true;
agent.interrupt();
agentInterrupt();
}
}
void readCommand()
{
if (postDebugTrapNeeded)
{
postDebugTrapNeeded = false;
agent.postDebugTrap();
}
try {
try {
if (postDebugTrapNeeded)
{
postDebugTrapNeeded = false;
try {
agent.postDebugTrap();
} catch (const FlycastException& e) {
throw Error(e.what());
}
}
std::string packet = recvPacket();
if (packet.empty())
return;
@ -539,7 +543,7 @@ private:
// Tell the remote stub about features supported by GDB,
// and query the stub for features it supports
char qsupported[128];
sprintf_s(qsupported, 128, "PacketSize=%i;vContSupported+", MAX_PACKET_LEN);
snprintf(qsupported, 128, "PacketSize=%i;vContSupported+", MAX_PACKET_LEN);
sendPacket(qsupported);
}
else if (pkt.rfind("qSymbol:", 0) == 0)
@ -626,7 +630,7 @@ private:
break;
}
default:
WARN_LOG(COMMON, "vCont action not supported %s", pkt[6], pkt.c_str());
WARN_LOG(COMMON, "vCont action not supported %s", pkt.c_str());
}
}
else if (pkt.rfind("vFile:", 0) == 0)
@ -667,15 +671,23 @@ private:
void step(u32 what = 0)
{
agent.step();
sendPacket("S05");
try {
agent.step();
sendPacket("S05");
} catch (const FlycastException& e) {
throw Error(e.what());
}
}
void stepRange(u32 from, u32 to)
{
sendPacket("OK");
agent.stepRange(from, to);
sendPacket("S05");
try {
sendPacket("OK");
agent.stepRange(from, to);
sendPacket("S05");
} catch (const FlycastException& e) {
throw Error(e.what());
}
}
void insertMatchpoint(const std::string& pkt)
@ -688,22 +700,23 @@ private:
sendPacket("E01");
}
switch (type) {
case DebugAgent::Breakpoint::Type::BP_TYPE_SOFTWARE_BREAK: // soft bp
if (agent.insertMatchpoint(0, addr, len))
case DebugAgent::Breakpoint::BP_TYPE_SOFTWARE_BREAK: // soft bp
if (agent.insertMatchpoint(DebugAgent::Breakpoint::BP_TYPE_SOFTWARE_BREAK,
addr, len))
sendPacket("OK");
else
sendPacket("E01");
break;
case DebugAgent::Breakpoint::Type::BP_TYPE_HARDWARE_BREAK: // hardware bp
case DebugAgent::Breakpoint::BP_TYPE_HARDWARE_BREAK: // hardware bp
sendPacket("");
break;
case DebugAgent::Breakpoint::Type::BP_TYPE_WRITE_WATCHPOINT: // write watchpoint
case DebugAgent::Breakpoint::BP_TYPE_WRITE_WATCHPOINT: // write watchpoint
sendPacket("");
break;
case DebugAgent::Breakpoint::Type::BP_TYPE_READ_WATCHPOINT: // read watchpoint
case DebugAgent::Breakpoint::BP_TYPE_READ_WATCHPOINT: // read watchpoint
sendPacket("");
break;
case DebugAgent::Breakpoint::Type::BP_TYPE_ACCESS_WATCHPOINT: // access watchpoint
case DebugAgent::Breakpoint::BP_TYPE_ACCESS_WATCHPOINT: // access watchpoint
sendPacket("");
break;
default:
@ -723,7 +736,8 @@ private:
}
switch (type) {
case 0: // soft bp
if (agent.removeMatchpoint(0, addr, len))
if (agent.removeMatchpoint(DebugAgent::Breakpoint::BP_TYPE_SOFTWARE_BREAK,
addr, len))
sendPacket("OK");
else
sendPacket("E01");
@ -748,7 +762,7 @@ private:
void interrupt()
{
u32 signal = agent.interrupt();
u32 signal = agentInterrupt();
char s[10];
sprintf(s, "S%02x", signal);
sendPacket(s);
@ -888,6 +902,15 @@ private:
throw Error("I/O error");
}
u32 agentInterrupt()
{
try {
return agent.interrupt();
} catch (const FlycastException& e) {
throw Error(e.what());
}
}
bool initialised = false;
bool stopRequested = false;
bool attached = false;
@ -932,8 +955,12 @@ static void emuEventCallback(Event event, void *)
switch (event)
{
case Event::Resume:
if (!gdbServer.isRunning())
gdbServer.run();
try {
if (!gdbServer.isRunning())
gdbServer.run();
} catch (const GdbServer::Error& e) {
ERROR_LOG(COMMON, "%s", e.what());
}
break;
case Event::Terminate:
gdbServer.stop();

View File

@ -624,7 +624,9 @@ void Emulator::runInternal()
void Emulator::unloadGame()
{
stop();
try {
stop();
} catch (...) { }
if (state == Loaded || state == Error)
{
if (state == Loaded && config::AutoSaveState && !settings.content.path.empty() && !settings.naomi.multiboard)
@ -676,11 +678,10 @@ void Emulator::stop()
{
rend_cancel_emu_wait();
try {
auto future = threadResult;
if(future.valid())
future.get();
checkStatus(true);
} catch (const FlycastException& e) {
WARN_LOG(COMMON, "%s", e.what());
throw e;
}
nvmem::saveFiles();
EventManager::event(Event::Pause);
@ -866,12 +867,11 @@ void Emulator::start()
TermAudio();
} catch (...) {
setNetworkState(false);
state = Error;
sh4_cpu.Stop();
TermAudio();
throw;
}
}).share();
});
}
else
{
@ -882,20 +882,24 @@ void Emulator::start()
EventManager::event(Event::Resume);
}
bool Emulator::checkStatus()
bool Emulator::checkStatus(bool wait)
{
try {
const std::lock_guard<std::mutex> lock(mutex);
if (threadResult.valid())
{
auto result = threadResult.wait_for(std::chrono::seconds(0));
if (result == std::future_status::timeout)
return true;
if (!wait)
{
auto result = threadResult.wait_for(std::chrono::seconds(0));
if (result == std::future_status::timeout)
return true;
}
threadResult.get();
}
return false;
} catch (...) {
EventManager::event(Event::Pause);
state = Error;
throw;
}
}

View File

@ -158,7 +158,7 @@ public:
void vblank();
private:
bool checkStatus();
bool checkStatus(bool wait = false);
void runInternal();
enum State {
@ -170,7 +170,7 @@ private:
Terminated,
};
State state = Uninitialized;
std::shared_future<void> threadResult;
std::future<void> threadResult;
bool resetRequested = false;
bool singleStep = false;
u64 startTime = 0;

View File

@ -66,7 +66,9 @@ int flycast_init(int argc, char* argv[])
void dc_exit()
{
emu.stop();
try {
emu.stop();
} catch (...) { }
mainui_stop();
}
@ -153,8 +155,6 @@ void dc_loadstate(int index)
u32 total_size = 0;
FILE *f = nullptr;
emu.stop();
std::string filename = hostfs::getSavestatePath(index, false);
RZipFile zipFile;
if (zipFile.Open(filename, false))

View File

@ -455,12 +455,18 @@ void gui_open_settings()
{
if (!ggpo::active())
{
gui_state = GuiState::Commands;
HideOSD();
emu.stop();
try {
emu.stop();
gui_state = GuiState::Commands;
} catch (const FlycastException& e) {
gui_stop_game(e.what());
}
}
else
{
chat.toggle();
}
}
else if (gui_state == GuiState::VJoyEdit)
{

View File

@ -61,7 +61,6 @@ bool mainui_rend_frame()
if (config::ProfilerEnabled && config::ProfilerDrawToGUI)
gui_display_profiler();
} catch (const FlycastException& e) {
emu.unloadGame();
gui_stop_game(e.what());
return false;
}

View File

@ -629,13 +629,15 @@ static int suspendEventFilter(void *userdata, SDL_Event *event)
if (event->type == SDL_APP_WILLENTERBACKGROUND)
{
gui_save();
if (gameRunning)
{
emu.stop();
if (config::AutoSaveState)
dc_savestate(config::SavestateSlot);
}
return 0;
if (gameRunning)
{
try {
emu.stop();
if (config::AutoSaveState)
dc_savestate(config::SavestateSlot);
} catch (const FlycastException& e) { }
}
return 0;
}
return 1;
}

View File

@ -286,15 +286,25 @@ extern "C" JNIEXPORT void JNICALL Java_com_reicast_emulator_emu_JNIdc_setExterna
gui_refresh_files();
}
static void stopEmu()
static bool stopEmu()
{
if (!emu.running())
{
game_started = false;
}
else
emu.stop();
{
try {
emu.stop();
} catch (const FlycastException& e) {
game_started = false;
return false;
}
}
// in single-threaded mode, stopping is delayed
while (game_started)
std::this_thread::sleep_for(std::chrono::milliseconds(1));
return true;
}
extern "C" JNIEXPORT void JNICALL Java_com_reicast_emulator_emu_JNIdc_setGameUri(JNIEnv *env,jobject obj,jstring fileName)
@ -346,9 +356,8 @@ extern "C" JNIEXPORT void JNICALL Java_com_reicast_emulator_emu_JNIdc_pause(JNIE
stopEmu();
gui_stop_game();
}
else if (emu.running())
else if (stopEmu())
{
stopEmu();
game_started = true; // restart when resumed
if (config::AutoSaveState)
dc_savestate(config::SavestateSlot);

View File

@ -59,7 +59,14 @@ static bool emulatorRunning;
// Use this method to pause ongoing tasks, disable timers, and throttle down OpenGL ES frame rates. Games should use this method to pause the game.
emulatorRunning = emu.running();
if (emulatorRunning)
emu.stop();
{
try {
emu.stop();
} catch (const FlycastException& e) {
emulatorRunning = false;
emu.unloadGame();
}
}
}
- (void)applicationDidEnterBackground:(UIApplication *)application

View File

@ -1025,9 +1025,13 @@ static void update_variables(bool first_startup)
if (wasThreadedRendering != config::ThreadedRendering)
{
config::ThreadedRendering = wasThreadedRendering;
emu.stop();
config::ThreadedRendering = !wasThreadedRendering;
emu.start();
try {
emu.stop();
config::ThreadedRendering = !wasThreadedRendering;
emu.start();
} catch (const FlycastException& e) {
ERROR_LOG(COMMON, "%s", e.what());
}
}
if (rotate_screen != (prevRotateScreen ^ rotate_game))
{
@ -2043,7 +2047,12 @@ size_t retro_serialize_size()
std::lock_guard<std::mutex> lock(mtx_serialization);
if (!first_run)
emu.stop();
try {
emu.stop();
} catch (const FlycastException& e) {
ERROR_LOG(COMMON, "%s", e.what());
return 0;
}
Serializer ser;
dc_serialize(ser);
@ -2059,7 +2068,12 @@ bool retro_serialize(void *data, size_t size)
std::lock_guard<std::mutex> lock(mtx_serialization);
if (!first_run)
emu.stop();
try {
emu.stop();
} catch (const FlycastException& e) {
ERROR_LOG(COMMON, "%s", e.what());
return false;
}
Serializer ser(data, size);
dc_serialize(ser);
@ -2075,7 +2089,12 @@ bool retro_unserialize(const void * data, size_t size)
std::lock_guard<std::mutex> lock(mtx_serialization);
if (!first_run)
emu.stop();
try {
emu.stop();
} catch (const FlycastException& e) {
ERROR_LOG(COMMON, "%s", e.what());
return false;
}
try {
Deserializer deser(data, size);