Skip to content

refactor: 입력 처리에 코루틴 기반 비동기 방식 추가 #73

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

Open
USKTEA wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor: 입력 처리에 코루틴 기반 비동기 방식 추가 #73

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 31 additions & 16 deletions 20260218-3files/src/tetris/Input.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,8 @@
package tetris

import java.util.concurrent.ConcurrentLinkedQueue

/**
* 터미널 raw 모드 키보드 입력 처리.
*
* Why raw 모드 + 별도 스레드?
* → System.in.read()는 blocking call.
* → 메인 스레드에서 호출하면 게임 루프 정지.
* → 별도 daemon 스레드에서 읽고 큐에 적재.
* → 상세: .claude/docs/PL-001-tetris/findings.md "입력 처리" 결정 참고
*
* 방향키 escape sequence:
* → ↑: ESC [ A (27, 91, 65)
* → ↓: ESC [ B (27, 91, 66)
* → →: ESC [ C (27, 91, 67)
* → ←: ESC [ D (27, 91, 68)
*/
import kotlinx.coroutines.*
import kotlinx.coroutines.channels.Channel
Comment on lines +4 to +5
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

코딩 가이드라인 위반: kotlinx-coroutines 외부 의존성 사용 불가

프로젝트 코딩 가이드라인에 명시된 바와 같이, 이 프로젝트는 순수 Kotlin 표준 라이브러리만 사용해야 하며 kotlinx-coroutines와 같은 외부 의존성 도입이 금지되어 있습니다. 타이밍 제어에는 Thread.sleep()을, 비동기 입력 처리에는 기존 Thread 기반 방식을 사용해야 합니다.

코루틴 기반 비동기 API 대신 기존 Thread + ConcurrentLinkedQueue 패턴을 확장하는 방식을 고려해 주세요.

As per coding guidelines: "Use pure Kotlin standard library only - do not introduce external dependencies like kotlinx-coroutines, use Thread.sleep() for timing control instead of ScheduledExecutor or Coroutines"

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@20260218-3files/src/tetris/Input.kt` around lines 4 - 5, The file imports
kotlinx.coroutines and Channel which violates the "pure Kotlin standard library"
rule; remove all kotlinx-coroutines imports and any uses of
CoroutineScope/launch/Channel in Input.kt and replace them with a Thread-based
producer/consumer using java.util.concurrent.ConcurrentLinkedQueue (or a
synchronized Kotlin MutableList) for queued key events and a dedicated input
Thread that polls the queue; use Thread.sleep(...) for timing control in any
polling loops and signal consumption via simple polling or wait/notify, and
update methods that referenced Channel.send/receive or coroutine jobs to
push/pop from the queue and to start/stop the input Thread instead.


enum class Action {
LEFT, RIGHT, ROTATE, SOFT_DROP, HARD_DROP, QUIT, NONE
Expand Down Expand Up @@ -86,4 +72,33 @@ class Input {
fun stop() {
running = false
}

private val scope = CoroutineScope(Dispatchers.IO)
private val channel = Channel<Action>(Channel.CONFLATED)
Comment on lines +76 to +77
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Channel 리소스 관리 누락

Channel이 선언되었지만 stopAsync()에서 scope.cancel()만 호출하고 channel.close()를 호출하지 않습니다. 가이드라인 위반과 별개로, 코루틴을 사용한다면 리소스 정리가 필요합니다.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@20260218-3files/src/tetris/Input.kt` around lines 76 - 77, The
Channel<Action> created as private val channel =
Channel<Action>(Channel.CONFLATED) isn't closed; update stopAsync() to call
channel.close() after cancelling the scope (or before) to properly release the
channel resource, and ensure any places that send to channel (uses of
channel.send/trySend) handle or avoid sends after stopAsync() to prevent
ClosedSendChannelException; reference the properties scope, channel and the
stopAsync() method when making the change.


fun startAsync() {
scope.launch {
while (isActive) {
val b = withContext(Dispatchers.IO) { System.`in`.read() }
if (b == -1) continue
val action = when (b) {
27 -> parseEscapeSequence()
32 -> Action.HARD_DROP
113, 81 -> Action.QUIT
else -> Action.NONE
}
if (action != Action.NONE) {
channel.send(action)
}
}
}
}
Comment on lines +79 to +95
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

parseEscapeSequence() 호출 시 블로킹 I/O 문제

가이드라인 위반 이슈와 별개로, 구현 자체에도 문제가 있습니다. Line 82에서 withContext(Dispatchers.IO)로 System.in.read()를 래핑했지만, Line 85에서 호출하는 parseEscapeSequence()는 내부적으로 System.in.read()를 직접 호출합니다. 이는 Dispatchers.IO 컨텍스트 밖에서 블로킹 I/O를 수행하게 됩니다.

만약 코루틴 사용이 허용된다면 수정 제안 
 fun startAsync() {
     scope.launch {
         while (isActive) {
-            val b = withContext(Dispatchers.IO) { System.`in`.read() }
-            if (b == -1) continue
-            val action = when (b) {
-                27 -> parseEscapeSequence()
-                32 -> Action.HARD_DROP
-                113, 81 -> Action.QUIT
-                else -> Action.NONE
-            }
+            val action = withContext(Dispatchers.IO) {
+                val b = System.`in`.read()
+                if (b == -1) return@withContext Action.NONE
+                when (b) {
+                    27 -> parseEscapeSequence()
+                    32 -> Action.HARD_DROP
+                    113, 81 -> Action.QUIT
+                    else -> Action.NONE
+                }
+            }
             if (action != Action.NONE) {
                 channel.send(action)
             }
         }
     }
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@20260218-3files/src/tetris/Input.kt` around lines 79 - 95, The call to
parseEscapeSequence() from startAsync() performs blocking System.in.read()
outside the IO dispatcher: make parseEscapeSequence a suspend function (or
ensure its internal blocking reads are wrapped) and run it inside the IO context
used for System.in.read(); specifically, either (A) change parseEscapeSequence
to suspend and run it via withContext(Dispatchers.IO) so all System.in.read()
calls execute on the IO dispatcher, or (B) modify parseEscapeSequence to wrap
its internal System.in.read() calls with withContext(Dispatchers.IO) (or accept
an injected Reader/BufferedInputStream already using the IO dispatcher) so no
blocking I/O happens on the main coroutine context; update startAsync to call
the suspend version accordingly and keep channel.send(...) behavior unchanged.


suspend fun pollAsync(): Action {
return channel.tryReceive().getOrNull() ?: Action.NONE
}
Comment on lines +97 to +99
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

불필요한 suspend 수정자

tryReceive()는 suspend 함수가 아니며 즉시 반환됩니다. 현재 구현에서 suspend 키워드는 불필요하며 호출자에게 혼란을 줄 수 있습니다. 기존 poll() 메서드와 동일하게 non-blocking 동작을 원한다면 suspend를 제거하세요.

수정 제안 
-    suspend fun pollAsync(): Action {
+    fun pollAsync(): Action {
         return channel.tryReceive().getOrNull() ?: Action.NONE
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
suspend fun pollAsync(): Action {
return channel.tryReceive().getOrNull() ?: Action.NONE
}
fun pollAsync(): Action {
return channel.tryReceive().getOrNull() ?: Action.NONE
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@20260218-3files/src/tetris/Input.kt` around lines 97 - 99, The pollAsync()
function is marked suspend but uses channel.tryReceive() which is
non-suspending, so remove the unnecessary suspend modifier on pollAsync to make
it a regular (non-suspending) function matching the non-blocking behavior of
poll(); update the function signature of pollAsync() (and any callers) to a
non-suspending function that returns Action and keep the existing body returning
channel.tryReceive().getOrNull() ?: Action.NONE.


fun stopAsync() {
scope.cancel()
}
Comment on lines +101 to +103
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

리소스 정리 불완전

scope.cancel()만 호출하고 channel.close()를 호출하지 않습니다. 완전한 정리를 위해 채널도 닫아야 합니다.

수정 제안 
     fun stopAsync() {
         scope.cancel()
+        channel.close()
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fun stopAsync() {
scope.cancel()
}
fun stopAsync() {
scope.cancel()
channel.close()
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@20260218-3files/src/tetris/Input.kt` around lines 101 - 103, In stopAsync(),
you only call scope.cancel() but must also close the coroutine Channel to fully
release resources; update the stopAsync() function to call channel.close() (or
channel.close(cause) if you want to surface the cancellation cause) after
cancelling the scope, and guard the close with a safe check/try-catch if the
channel may already be closed; ensure you reference stopAsync(), scope.cancel(),
and channel.close() in the change.

}