From d4adabda9380c21817d15ae6d871db8d0ed44d30 Mon Sep 17 00:00:00 2001 From: Aaron Culliney Date: Fri, 30 Oct 2015 23:15:53 -0700 Subject: [PATCH] SNEAKY BUGFIX race condition between UI thread and CPU thread on disk insertion - Previously we would start the CPU thread and then insert, which has been a long standing race condition, but possibly better exposed since the recent disk.c rephactor to use mmap I/O - This directly manifested on a Kindle Fire 1st Gen as a crash, and, anecdotally on other devices as a "stalled" disk read requiring a reboot of the virtual machine. Yay for crappy devices helping to expose crappy code! (git blame me). =P --- .../src/main/java/org/deadc0de/apple2ix/Apple2DisksMenu.java | 4 ++-- Android/jni/jnihooks.c | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Android/app/src/main/java/org/deadc0de/apple2ix/Apple2DisksMenu.java b/Android/app/src/main/java/org/deadc0de/apple2ix/Apple2DisksMenu.java index c45f0999..fa64fe84 100644 --- a/Android/app/src/main/java/org/deadc0de/apple2ix/Apple2DisksMenu.java +++ b/Android/app/src/main/java/org/deadc0de/apple2ix/Apple2DisksMenu.java @@ -615,8 +615,6 @@ public class Apple2DisksMenu implements Apple2MenuView { builder.setPositiveButton(R.string.ok, new DialogInterface.OnClickListener() { @Override public void onClick(DialogInterface dialog, int which) { - dialog.dismiss(); - mActivity.dismissAllMenus(); boolean isDriveA = diskA.isChecked(); boolean diskReadOnly = readOnly.isChecked(); if (isDriveA) { @@ -626,6 +624,8 @@ public class Apple2DisksMenu implements Apple2MenuView { Apple2Preferences.CURRENT_DISK_B_RO.saveBoolean(mActivity, diskReadOnly); Apple2Preferences.CURRENT_DISK_B.saveString(mActivity, imageName); } + dialog.dismiss(); + mActivity.dismissAllMenus(); } }); diff --git a/Android/jni/jnihooks.c b/Android/jni/jnihooks.c index 373d0405..b8d60045 100644 --- a/Android/jni/jnihooks.c +++ b/Android/jni/jnihooks.c @@ -311,6 +311,8 @@ void Java_org_deadc0de_apple2ix_Apple2Activity_nativeChooseDisk(JNIEnv *env, job int drive = driveA ? 0 : 1; int ro = readOnly ? 1 : 0; + assert(cpu_isPaused() && "considered dangerous to insert disk image when CPU thread is running"); + LOG(": (%s, %s, %s)", path, driveA ? "drive A" : "drive B", readOnly ? "read only" : "read/write"); if (disk6_insert(drive, path, ro)) { char *gzPath = NULL;