diff --git a/mosaic-tty-terminal/src/commonTest/kotlin/com/jakewharton/mosaic/tty/terminal/BaseEventParserTest.kt b/mosaic-tty-terminal/src/commonTest/kotlin/com/jakewharton/mosaic/tty/terminal/BaseEventParserTest.kt index 06ec5ed7..b2da6f5c 100644 --- a/mosaic-tty-terminal/src/commonTest/kotlin/com/jakewharton/mosaic/tty/terminal/BaseEventParserTest.kt +++ b/mosaic-tty-terminal/src/commonTest/kotlin/com/jakewharton/mosaic/tty/terminal/BaseEventParserTest.kt @@ -7,7 +7,7 @@ import kotlin.test.AfterTest import kotlin.test.BeforeTest abstract class BaseEventParserTest { - internal val testTty = TestTty.create() + internal val testTty = TestTty.bind() private val tty = testTty.tty internal val parser = EventParser(tty) diff --git a/mosaic-tty-terminal/src/commonTest/kotlin/com/jakewharton/mosaic/tty/terminal/TerminalTester.kt b/mosaic-tty-terminal/src/commonTest/kotlin/com/jakewharton/mosaic/tty/terminal/TerminalTester.kt index 63c6f723..a0a4284d 100644 --- a/mosaic-tty-terminal/src/commonTest/kotlin/com/jakewharton/mosaic/tty/terminal/TerminalTester.kt +++ b/mosaic-tty-terminal/src/commonTest/kotlin/com/jakewharton/mosaic/tty/terminal/TerminalTester.kt @@ -23,7 +23,7 @@ import kotlinx.io.unsafe.UnsafeBufferOperations fun terminalTest(block: suspend TerminalTester.() -> Unit) { runBlocking { - TestTty.create().use { testTty -> + TestTty.bind().use { testTty -> TerminalTester(testTty).block() } } diff --git a/mosaic-tty/api/mosaic-tty.api b/mosaic-tty/api/mosaic-tty.api index 700b2914..3c962aea 100644 --- a/mosaic-tty/api/mosaic-tty.api +++ b/mosaic-tty/api/mosaic-tty.api @@ -1,8 +1,8 @@ public final class com/jakewharton/mosaic/tty/TestTty : java/lang/AutoCloseable { public static final field Companion Lcom/jakewharton/mosaic/tty/TestTty$Companion; public synthetic fun (JLcom/jakewharton/mosaic/tty/Tty;Lkotlin/jvm/internal/DefaultConstructorMarker;)V + public static final fun bind ()Lcom/jakewharton/mosaic/tty/TestTty; public fun close ()V - public static final fun create ()Lcom/jakewharton/mosaic/tty/TestTty; public final fun focusEvent (Z)V public final fun getTty ()Lcom/jakewharton/mosaic/tty/Tty; public final fun interruptRead ()V @@ -14,7 +14,7 @@ public final class com/jakewharton/mosaic/tty/TestTty : java/lang/AutoCloseable } public final class com/jakewharton/mosaic/tty/TestTty$Companion { - public final fun create ()Lcom/jakewharton/mosaic/tty/TestTty; + public final fun bind ()Lcom/jakewharton/mosaic/tty/TestTty; } public final class com/jakewharton/mosaic/tty/Tty : java/lang/AutoCloseable { diff --git a/mosaic-tty/api/mosaic-tty.klib.api b/mosaic-tty/api/mosaic-tty.klib.api index 3786339d..f2eb41b3 100644 --- a/mosaic-tty/api/mosaic-tty.klib.api +++ b/mosaic-tty/api/mosaic-tty.klib.api @@ -24,7 +24,7 @@ final class com.jakewharton.mosaic.tty/TestTty : kotlin/AutoCloseable { // com.j final fun write(kotlin/ByteArray, kotlin/Int, kotlin/Int): kotlin/Int // com.jakewharton.mosaic.tty/TestTty.write|write(kotlin.ByteArray;kotlin.Int;kotlin.Int){}[0] final object Companion { // com.jakewharton.mosaic.tty/TestTty.Companion|null[0] - final fun create(): com.jakewharton.mosaic.tty/TestTty // com.jakewharton.mosaic.tty/TestTty.Companion.create|create(){}[0] + final fun bind(): com.jakewharton.mosaic.tty/TestTty // com.jakewharton.mosaic.tty/TestTty.Companion.bind|bind(){}[0] } } diff --git a/mosaic-tty/src/commonMain/c/mosaic-test-tty-posix.c b/mosaic-tty/src/commonMain/c/mosaic-test-tty-posix.c index 92a25d76..12f7f196 100644 --- a/mosaic-tty/src/commonMain/c/mosaic-test-tty-posix.c +++ b/mosaic-tty/src/commonMain/c/mosaic-test-tty-posix.c @@ -49,8 +49,9 @@ MosaicTestTtyInitResult testTty_init() { } MosaicTtyInitResult ttyInitResult = tty_initWithFd(childFd); - if (unlikely(ttyInitResult.error)) { + if (unlikely(!ttyInitResult.tty)) { result.error = ttyInitResult.error; + result.already_bound = ttyInitResult.already_bound; goto err_child; } diff --git a/mosaic-tty/src/commonMain/c/mosaic-test-tty-windows.c b/mosaic-tty/src/commonMain/c/mosaic-test-tty-windows.c index c0a23fa6..b4fd3efa 100644 --- a/mosaic-tty/src/commonMain/c/mosaic-test-tty-windows.c +++ b/mosaic-tty/src/commonMain/c/mosaic-test-tty-windows.c @@ -14,11 +14,11 @@ typedef struct MosaicTestTtyImpl { atomic_bool interrupt; } MosaicTestTtyImpl; +static atomic_flag globalTestTty = ATOMIC_FLAG_INIT; + MosaicTestTtyInitResult testTty_init() { MosaicTestTtyInitResult result = {}; - // TODO Atomic boolean guaranteeing single instance - MosaicTestTtyImpl *testTty = calloc(1, sizeof(MosaicTestTtyImpl)); if (unlikely(testTty == NULL)) { // result.testTty is set to 0 which will trigger OOM. @@ -46,10 +46,9 @@ MosaicTestTtyInitResult testTty_init() { } MosaicTtyInitResult ttyInitResult = tty_initWithHandles(conin, conoutWrite, true); - if (unlikely(ttyInitResult.error)) { - CloseHandle(conoutRead); - CloseHandle(conoutWrite); + if (unlikely(!ttyInitResult.tty)) { result.error = ttyInitResult.error; + result.already_bound = ttyInitResult.already_bound; goto err_conout; } @@ -59,6 +58,14 @@ MosaicTestTtyInitResult testTty_init() { result.testTty = testTty; + if (unlikely(atomic_flag_test_and_set(&globalTestTty))) { + // We initialized an instance but there already was a global instance. + result.testTty = NULL; + result.error = tty_free(ttyInitResult.tty); + result.already_bound = true; + goto err_conout; + } + ret: return result; @@ -188,6 +195,7 @@ uint32_t testTty_free(MosaicTestTty *testTty) { result = GetLastError(); } + atomic_flag_clear(&globalTestTty); free(testTty); return result; } diff --git a/mosaic-tty/src/commonMain/c/mosaic-test-tty.h b/mosaic-tty/src/commonMain/c/mosaic-test-tty.h index 58cf6e08..65a4e4f1 100644 --- a/mosaic-tty/src/commonMain/c/mosaic-test-tty.h +++ b/mosaic-tty/src/commonMain/c/mosaic-test-tty.h @@ -10,6 +10,7 @@ typedef struct MosaicTestTtyImpl MosaicTestTty; typedef struct MosaicTestTtyInitResult { MosaicTestTty *testTty; uint32_t error; + bool already_bound; } MosaicTestTtyInitResult; MosaicTestTtyInitResult testTty_init(); diff --git a/mosaic-tty/src/commonMain/c/mosaic-tty-posix.c b/mosaic-tty/src/commonMain/c/mosaic-tty-posix.c index 139eba01..6854b900 100644 --- a/mosaic-tty/src/commonMain/c/mosaic-tty-posix.c +++ b/mosaic-tty/src/commonMain/c/mosaic-tty-posix.c @@ -14,6 +14,8 @@ #include #include +static _Atomic(MosaicTty *) globalTty; + MosaicTtyInitResult tty_initWithFd(int fd) { MosaicTtyInitResult result = {}; @@ -35,29 +37,6 @@ MosaicTtyInitResult tty_initWithFd(int fd) { result.tty = tty; - ret: - return result; - - err: - free(tty); - goto ret; -} - -static _Atomic(MosaicTty *) globalTty; - -MosaicTtyInitResult tty_init() { - MosaicTtyInitResult result = {}; - - int fd = open("/dev/tty", O_RDWR); - if (unlikely(fd == -1)) { - result.error = errno; - result.no_tty = true; - goto ret; - } - - result = tty_initWithFd(fd); - - MosaicTty *tty = result.tty; MosaicTty *expected = NULL; if (likely(tty) && !atomic_compare_exchange_strong(&globalTty, &expected, tty)) { // We initialized an instance but there already was a global instance. @@ -68,6 +47,22 @@ MosaicTtyInitResult tty_init() { ret: return result; + + err: + free(tty); + goto ret; +} + +MosaicTtyInitResult tty_init() { + int fd = open("/dev/tty", O_RDWR); + if (likely(fd != -1)) { + return tty_initWithFd(fd); + } + + MosaicTtyInitResult result = {}; + result.error = errno; + result.no_tty = true; + return result; } void tty_setCallback(MosaicTty *tty, MosaicTtyCallback *callback) { @@ -173,6 +168,8 @@ void sigwinchHandler(int value UNUSED) { } else { // TODO Send errno somewhere? Maybe once we get debug logs working. } + } else { + // TODO Send warning somewhere? Maybe once we get debug logs working. } } diff --git a/mosaic-tty/src/commonMain/kotlin/com/jakewharton/mosaic/tty/TestTty.kt b/mosaic-tty/src/commonMain/kotlin/com/jakewharton/mosaic/tty/TestTty.kt index 489e7c1b..ee5de5ee 100644 --- a/mosaic-tty/src/commonMain/kotlin/com/jakewharton/mosaic/tty/TestTty.kt +++ b/mosaic-tty/src/commonMain/kotlin/com/jakewharton/mosaic/tty/TestTty.kt @@ -2,7 +2,16 @@ package com.jakewharton.mosaic.tty public expect class TestTty : AutoCloseable { public companion object { - public fun create(): TestTty + + /** + * Initialize a [TestTty] instance. Only a single [TestTty] instance can be bound at a time, + * and only when a [Tty] is not also bound. Subsequent calls will throw an exception until + * [TestTty.close] is called. + * + * @throws IOException If an error occurred creating the PTY. + * @throws IllegalStateException If another instance is already bound. + */ + public fun bind(): TestTty } public val tty: Tty diff --git a/mosaic-tty/src/commonTest/kotlin/com/jakewharton/mosaic/tty/TestTtyTest.kt b/mosaic-tty/src/commonTest/kotlin/com/jakewharton/mosaic/tty/TestTtyTest.kt index 54d067bf..156bf1cf 100644 --- a/mosaic-tty/src/commonTest/kotlin/com/jakewharton/mosaic/tty/TestTtyTest.kt +++ b/mosaic-tty/src/commonTest/kotlin/com/jakewharton/mosaic/tty/TestTtyTest.kt @@ -1,7 +1,10 @@ package com.jakewharton.mosaic.tty +import assertk.assertFailure import assertk.assertThat +import assertk.assertions.hasMessage import assertk.assertions.isEqualTo +import assertk.assertions.isInstanceOf import assertk.assertions.isZero import kotlin.test.AfterTest import kotlin.test.BeforeTest @@ -13,7 +16,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.test.runTest class TestTtyTest { - private val testTty = TestTty.create() + private val testTty = TestTty.bind() private val tty = testTty.tty @BeforeTest fun before() { @@ -26,15 +29,11 @@ class TestTtyTest { testTty.close() } - @Test fun canCreateMultiple() { - if (isWindows()) return // TODO Not currently supported. - - TestTty.create().use { testTty2 -> - testTty.write("hey\n") - testTty2.write("bye\n") - assertThat(testTty2.tty.read(4)).isEqualTo("bye\n") - assertThat(testTty.tty.read(4)).isEqualTo("hey\n") - } + @Test fun onlyOne() { + assertFailure { + TestTty.bind() + }.isInstanceOf() + .hasMessage("TestTty or Tty already bound") } @Test fun multipleRawModeResetCycles() { diff --git a/mosaic-tty/src/commonTest/kotlin/com/jakewharton/mosaic/tty/TtyTest.kt b/mosaic-tty/src/commonTest/kotlin/com/jakewharton/mosaic/tty/TtyTest.kt index e85a17e0..a61bd8fd 100644 --- a/mosaic-tty/src/commonTest/kotlin/com/jakewharton/mosaic/tty/TtyTest.kt +++ b/mosaic-tty/src/commonTest/kotlin/com/jakewharton/mosaic/tty/TtyTest.kt @@ -18,7 +18,7 @@ import kotlinx.coroutines.test.runTest class TtyTest { private val events = ArrayDeque() - private val testTty = TestTty.create() + private val testTty = TestTty.bind() private val tty = testTty.tty @BeforeTest fun before() { diff --git a/mosaic-tty/src/jvmMain/c/com_jakewharton_mosaic_tty_Jni.c b/mosaic-tty/src/jvmMain/c/com_jakewharton_mosaic_tty_Jni.c index f4836e62..9d45658e 100644 --- a/mosaic-tty/src/jvmMain/c/com_jakewharton_mosaic_tty_Jni.c +++ b/mosaic-tty/src/jvmMain/c/com_jakewharton_mosaic_tty_Jni.c @@ -360,14 +360,20 @@ Java_com_jakewharton_mosaic_tty_Jni_testTtyInit( jclass type UNUSED ) { MosaicTestTtyInitResult result = testTty_init(); - if (likely(!result.error)) { + if (likely(result.testTty)) { return (jlong) result.testTty; } - // This throw can fail, but the only condition that should cause that is OOM which - // will occur from returning 0 (which is otherwise ignored if the throw succeeds). - throwIoe(env, result.error); - return 0; + if (result.already_bound) { + jclass ise = (*env)->FindClass(env, "java/lang/IllegalStateException"); + (*env)->ThrowNew(env, ise, "TestTty or Tty already bound"); + } else if (result.error) { + throwIoe(env, result.error); + } else { + jclass ise = (*env)->FindClass(env, "java/lang/OutOfMemoryException"); + (*env)->ThrowNew(env, ise, NULL); + } + return 0; // Unused } JNIEXPORT jint JNICALL diff --git a/mosaic-tty/src/jvmMain/kotlin/com/jakewharton/mosaic/tty/TestTty.kt b/mosaic-tty/src/jvmMain/kotlin/com/jakewharton/mosaic/tty/TestTty.kt index 2d30c74d..718da9b8 100644 --- a/mosaic-tty/src/jvmMain/kotlin/com/jakewharton/mosaic/tty/TestTty.kt +++ b/mosaic-tty/src/jvmMain/kotlin/com/jakewharton/mosaic/tty/TestTty.kt @@ -10,14 +10,11 @@ public actual class TestTty private constructor( public actual companion object { @JvmStatic @Throws(IOException::class) - public actual fun create(): TestTty { + public actual fun bind(): TestTty { val testTtyPtr = testTtyInit() - if (testTtyPtr != 0L) { - val ttyPtr = testTtyGetTty(testTtyPtr) - val tty = Tty(ttyPtr) - return TestTty(testTtyPtr, tty) - } - throw OutOfMemoryError() + val ttyPtr = testTtyGetTty(testTtyPtr) + val tty = Tty(ttyPtr) + return TestTty(testTtyPtr, tty) } } diff --git a/mosaic-tty/src/nativeMain/kotlin/com/jakewharton/mosaic/tty/TestTty.kt b/mosaic-tty/src/nativeMain/kotlin/com/jakewharton/mosaic/tty/TestTty.kt index 8fbb7de7..b010322f 100644 --- a/mosaic-tty/src/nativeMain/kotlin/com/jakewharton/mosaic/tty/TestTty.kt +++ b/mosaic-tty/src/nativeMain/kotlin/com/jakewharton/mosaic/tty/TestTty.kt @@ -10,10 +10,13 @@ public actual class TestTty private constructor( public actual val tty: Tty, ) : AutoCloseable { public actual companion object { - public actual fun create(): TestTty { + public actual fun bind(): TestTty { val testTtyPtr = testTty_init().useContents { testTty?.let { return@useContents it } + if (already_bound) { + throw IllegalStateException("TestTty or Tty already bound") + } if (error != 0U) { throwIoe(error) }