From 557b90a39f60f4c4c369f6d3491edd699122d302 Mon Sep 17 00:00:00 2001 From: Rekai Musuka Date: Thu, 23 Nov 2023 00:49:40 -0600 Subject: [PATCH] fix: don't insta-crash due to an awful channel impl --- src/core/emu.zig | 45 +++++++++++++++++++++++----------------- src/main.zig | 30 ++++++++++++++------------- src/platform.zig | 54 ++++++++++++++++-------------------------------- 3 files changed, 60 insertions(+), 69 deletions(-) diff --git a/src/core/emu.zig b/src/core/emu.zig index f88604e..f972927 100644 --- a/src/core/emu.zig +++ b/src/core/emu.zig @@ -6,7 +6,6 @@ const Scheduler = @import("scheduler.zig").Scheduler; const Arm7tdmi = @import("arm32").Arm7tdmi; const Bus = @import("Bus.zig"); const Tracker = @import("../util.zig").FpsTracker; -const Channel = @import("zba-util").Channel(Message, 0x100); pub const Message = enum { Pause, Resume, Quit }; @@ -15,6 +14,18 @@ const isHalted = @import("cpu_util.zig").isHalted; const Timer = std.time.Timer; +pub const Synchro = struct { + const AtomicBool = std.atomic.Atomic(bool); + + // UI -> Emulator + ui_busy: *AtomicBool, + paused: *AtomicBool, // FIXME: can ui_busy and paused be the same? + should_quit: *AtomicBool, + + // Emulator -> UI + did_pause: *AtomicBool, +}; + /// 4 Cycles in 1 dot const cycles_per_dot = 4; @@ -41,18 +52,18 @@ const RunKind = enum { LimitedFPS, }; -pub fn run(cpu: *Arm7tdmi, scheduler: *Scheduler, tracker: *Tracker, rx: Channel.Receiver) void { +pub fn run(cpu: *Arm7tdmi, scheduler: *Scheduler, tracker: *Tracker, sync: Synchro) void { const audio_sync = config.config().guest.audio_sync and !config.config().host.mute; if (audio_sync) log.info("Audio sync enabled", .{}); if (config.config().guest.video_sync) { - inner(.LimitedFPS, audio_sync, cpu, scheduler, tracker, rx); + inner(.LimitedFPS, audio_sync, cpu, scheduler, tracker, sync); } else { - inner(.UnlimitedFPS, audio_sync, cpu, scheduler, tracker, rx); + inner(.UnlimitedFPS, audio_sync, cpu, scheduler, tracker, sync); } } -fn inner(comptime kind: RunKind, audio_sync: bool, cpu: *Arm7tdmi, scheduler: *Scheduler, tracker: ?*Tracker, rx: Channel.Receiver) void { +fn inner(comptime kind: RunKind, audio_sync: bool, cpu: *Arm7tdmi, scheduler: *Scheduler, tracker: ?*Tracker, sync: Synchro) void { if (kind == .UnlimitedFPS or kind == .LimitedFPS) { std.debug.assert(tracker != null); log.info("FPS tracking enabled", .{}); @@ -60,19 +71,16 @@ fn inner(comptime kind: RunKind, audio_sync: bool, cpu: *Arm7tdmi, scheduler: *S const bus_ptr: *Bus = @ptrCast(@alignCast(cpu.bus.ptr)); - var paused: bool = false; - switch (kind) { .Unlimited, .UnlimitedFPS => { log.info("Emulation w/out video sync", .{}); - while (true) { - if (rx.recv()) |m| switch (m) { - .Quit => break, - .Resume, .Pause => paused = m == .Pause, - }; + while (!sync.should_quit.load(.Monotonic)) { + if (sync.ui_busy.load(.Monotonic) or sync.paused.load(.Monotonic)) { + sync.did_pause.store(true, .Monotonic); - if (paused) continue; + continue; + } runFrame(scheduler, cpu); audioSync(audio_sync, bus_ptr.apu.stream, &bus_ptr.apu.is_buffer_full); @@ -85,13 +93,12 @@ fn inner(comptime kind: RunKind, audio_sync: bool, cpu: *Arm7tdmi, scheduler: *S var timer = Timer.start() catch @panic("failed to initalize std.timer.Timer"); var wake_time: u64 = frame_period; - while (true) { - if (rx.recv()) |m| switch (m) { - .Quit => break, - .Resume, .Pause => paused = m == .Pause, - }; + while (!sync.should_quit.load(.Monotonic)) { + if (sync.ui_busy.load(.Monotonic) or sync.paused.load(.Monotonic)) { + sync.did_pause.store(true, .Release); - if (paused) continue; + continue; + } runFrame(scheduler, cpu); const new_wake_time = videoSync(&timer, wake_time); diff --git a/src/main.zig b/src/main.zig index 7548c6b..fa44aa3 100644 --- a/src/main.zig +++ b/src/main.zig @@ -6,7 +6,7 @@ const clap = @import("clap"); const config = @import("config.zig"); const emu = @import("core/emu.zig"); -const Channel = @import("zba-util").Channel(emu.Message, 0x100); +const Synchro = @import("core/emu.zig").Synchro; const Gui = @import("platform.zig").Gui; const Bus = @import("core/Bus.zig"); const Scheduler = @import("core/scheduler.zig").Scheduler; @@ -103,10 +103,16 @@ pub fn main() void { var gui = Gui.init(allocator, &bus.apu, title_ptr) catch |e| exitln("failed to init gui: {}", .{e}); defer gui.deinit(); - var quit = std.atomic.Atomic(bool).init(false); + var sync: Synchro = blk: { + const AtomicBool = std.atomic.Atomic(bool); - var ch = Channel.init(allocator) catch |e| exitln("failed to initialize ui -> emu thread message channel: {}", .{e}); - defer ch.deinit(allocator); + var ui_busy = AtomicBool.init(false); + var paused = AtomicBool.init(true); + var should_quit = AtomicBool.init(false); + var did_pause = AtomicBool.init(false); + + break :blk .{ .ui_busy = &ui_busy, .paused = &paused, .should_quit = &should_quit, .did_pause = &did_pause }; + }; if (result.args.gdb != 0) { const Server = @import("gdbstub").Server; @@ -123,29 +129,25 @@ pub fn main() void { log.info("Starting GDB Server Thread", .{}); - const thread = std.Thread.spawn(.{}, Server.run, .{ &server, allocator, &quit }) catch |e| exitln("gdb server thread crashed: {}", .{e}); + const thread = std.Thread.spawn(.{}, Server.run, .{ &server, allocator, sync.should_quit }) catch |e| exitln("gdb server thread crashed: {}", .{e}); defer thread.join(); - gui.run(.Debug, .{ + gui.run(.{ .cpu = &cpu, .scheduler = &scheduler, - .ch = ch.tx, + .sync = sync, }) catch |e| exitln("main thread panicked: {}", .{e}); } else { var tracker = FpsTracker.init(); - // emu should start paused if there's no ROM to run - if (paths.rom == null) - ch.tx.send(.Pause); - - const thread = std.Thread.spawn(.{}, emu.run, .{ &cpu, &scheduler, &tracker, ch.rx }) catch |e| exitln("emu thread panicked: {}", .{e}); + const thread = std.Thread.spawn(.{}, emu.run, .{ &cpu, &scheduler, &tracker, sync }) catch |e| exitln("emu thread panicked: {}", .{e}); defer thread.join(); - gui.run(.Standard, .{ + gui.run(.{ .cpu = &cpu, .scheduler = &scheduler, - .ch = ch.tx, .tracker = &tracker, + .sync = sync, }) catch |e| exitln("main thread panicked: {}", .{e}); } } diff --git a/src/platform.zig b/src/platform.zig index f2be651..c5ddac2 100644 --- a/src/platform.zig +++ b/src/platform.zig @@ -12,7 +12,7 @@ const Arm7tdmi = @import("arm32").Arm7tdmi; const Bus = @import("core/Bus.zig"); const Scheduler = @import("core/scheduler.zig").Scheduler; const FpsTracker = @import("util.zig").FpsTracker; -const Channel = @import("zba-util").Channel(emu.Message, 0x100); +const Synchro = @import("core/emu.zig").Synchro; const KeyInput = @import("core/bus/io.zig").KeyInput; const gba_width = @import("core/ppu.zig").width; @@ -95,18 +95,16 @@ pub const Gui = struct { } const RunOptions = struct { - ch: Channel.Sender, + sync: Synchro, tracker: ?*FpsTracker = null, cpu: *Arm7tdmi, scheduler: *Scheduler, }; - const RunMode = enum { Standard, Debug }; - - pub fn run(self: *Self, comptime mode: RunMode, opt: RunOptions) !void { + pub fn run(self: *Self, opt: RunOptions) !void { const cpu = opt.cpu; const tracker = opt.tracker; - const ch = opt.ch; + const sync = opt.sync; const bus_ptr: *Bus = @ptrCast(@alignCast(cpu.bus.ptr)); @@ -196,50 +194,34 @@ pub const Gui = struct { switch (self.state.emulation) { .Transition => |inner| switch (inner) { .Active => { - ch.send(.Resume); + sync.paused.store(false, .Monotonic); if (!config.config().host.mute) SDL.SDL_PauseAudioDevice(self.audio.device, 0); self.state.emulation = .Active; }, .Inactive => { // Assert that double pausing is impossible - SDL.SDL_PauseAudioDevice(self.audio.device, 1); - ch.send(.Pause); + sync.paused.store(true, .Monotonic); self.state.emulation = .Inactive; }, }, - .Active => { - const is_std = mode == .Standard; + .Active => blk: { + sync.ui_busy.store(true, .Monotonic); + defer sync.ui_busy.store(false, .Monotonic); - if (is_std) ch.send(.Pause); - defer if (is_std) ch.send(.Resume); + // spin until we know the emu is paused :) - // switch (mode) { - // .Standard => blk: { - // const limit = 15; // TODO: What should this be? + const timeout = 0x100000; + wait_loop: for (0..timeout) |i| { + const ret = sync.did_pause.compareAndSwap(true, false, .Acquire, .Monotonic); + if (ret == null) break :wait_loop; - // // TODO: learn more about std.atomic.spinLoopHint(); - // for (0..limit) |_| { - // const message = channel.gui.pop() orelse continue; + if (i == timeout - 1) break :blk; + } - // switch (message) { - // .Paused => break :blk, - // .Quit => unreachable, - // } - // } - - // log.info("timed out waiting for emu thread to pause (limit: {})", .{limit}); - // break :skip_draw; - // }, - // .Debug => blk: { - // switch (channel.gui.pop() orelse break :blk) { - // .Paused => unreachable, // only in standard mode - // .Quit => break :emu_loop, // FIXME: gdb side of emu is seriously out-of-date... - // } - // }, - // } + // while (sync.did_pause.compareAndSwap(true, false, .Acquire, .Acquire) != null) std.atomic.spinLoopHint(); // Add FPS count to the histogram if (tracker) |t| self.state.fps_hist.push(t.value()) catch {}; @@ -278,7 +260,7 @@ pub const Gui = struct { SDL.SDL_GL_SwapWindow(self.window); } - ch.send(.Quit); + sync.should_quit.store(true, .Monotonic); } fn glGetProcAddress(ctx: SDL.SDL_GLContext, proc: [:0]const u8) ?*anyopaque {