Skip to content

Add tryWait() to Child class #21

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
slomkowski wants to merge 2 commits into
base: main
Choose a base branch
from
+111 −27
Open

Add tryWait() to Child class #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8dd1b34
Add tryWait() to Child object
slomkowski May 6, 2025
a43619c
Apply nitpicks from Copilot
slomkowski Apr 16, 2026
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
2 changes: 2 additions & 0 deletions kommand-core/kommand_core.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ unsigned int id_child(const void *child);

struct IntResult wait_child(const void *child);

struct IntResult try_wait_child(const void *child);

/**
* The returned [Output] will empty
* if convert the [Child.stdout] and [Child.stderr] to buffered
Expand Down
15 changes: 13 additions & 2 deletions kommand-core/src/bin/leaks_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ use std::ffi::{c_char, c_ulonglong};

use kommand_core::ffi_util::{as_cstring, into_string};
use kommand_core::io::{drop_stdout, read_line_stdout};
use kommand_core::process::{buffered_stdout_child, Stdio};
use kommand_core::process::{buffered_stdout_child, try_wait_child, Stdio};
use kommand_core::process::{drop_child, wait_child};
use kommand_core::process::{drop_command, new_command, spawn_command, stdout_command};
use kommand_core::result::ErrorType;

fn main() {
unsafe {
(0..1).for_each(|i| {
(0..100).for_each(|i| {
let command = new_command(as_cstring("target/debug/kommand-echo").as_ptr());
// arg_command(command, as_cstring("echo").as_ptr());
stdout_command(command, Stdio::Pipe);
Expand All @@ -21,6 +21,12 @@ fn main() {
panic!("{}", into_string(result.err))
};

if try_wait_child(child).ok == -2 {
println!("Child is not finished");
} else {
println!("Child is finished");
}

let stdout = buffered_stdout_child(child);
if stdout.is_null() {
panic!("stdout is null");
Expand All @@ -44,6 +50,11 @@ fn main() {
}

wait_child(child);
let status = try_wait_child(child);
if status.ok < 0 {
drop_command(command);
panic!("No proper status code returned: {}", status.ok);
}
drop_stdout(stdout);
drop_child(child);
drop_command(command);
Expand Down
6 changes: 6 additions & 0 deletions kommand-core/src/process/child.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ pub extern "C" fn wait_child(mut child: *const c_void) -> IntResult {
child.wait().into()
}

#[no_mangle]
pub extern "C" fn try_wait_child(mut child: *const c_void) -> IntResult {
let child = as_child_mut(&mut child);
child.try_wait().into()
}

/// The returned [Output] will empty
/// if convert the [Child.stdout] and [Child.stderr] to buffered
/// with [buffered_stdout_child] and [buffered_stderr_child]
Expand Down
31 changes: 24 additions & 7 deletions kommand-core/src/result.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,16 +117,33 @@ impl<E: Display> From<Result<Option<String>, E>> for VoidResult {
impl From<io::Result<std::process::ExitStatus>> for IntResult {
fn from(value: io::Result<std::process::ExitStatus>) -> Self {
match value {
Ok(status) => match status.code() {
Ok(status) => Ok(Some(status)),
Err(e) => Err(e),
}
.into()
Comment on lines +120 to +123

@BppleMan BppleMan May 7, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Ok(status) => Ok(Some(status)),
Err(e) => Err(e),
}
.into()
value.map_or_else(Err, |status| Ok(Some(status))).into()

}
}

impl From<io::Result<Option<std::process::ExitStatus>>> for IntResult {
fn from(value: io::Result<Option<std::process::ExitStatus>>) -> Self {
match value {
Ok(status) => match status {
None => IntResult {
ok: -1,
err: into_cstring("No exit code"),
ok: -2,
err: into_cstring("Application still running"),
error_type: ErrorType::None,
Comment on lines +132 to 134

@BppleMan BppleMan May 7, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add new ErrorType variant for "Application still running"

},
Some(code) => IntResult {
ok: code,
err: std::ptr::null_mut() as *mut c_char,
error_type: ErrorType::None,
Some(status) => match status.code() {
None => IntResult {
ok: -1,
err: into_cstring("No exit code"),
error_type: ErrorType::None,
},
Some(code) => IntResult {
ok: code,
err: std::ptr::null_mut() as *mut c_char,
error_type: ErrorType::None,
},
},
},
Err(e) => IntResult {
Expand Down
3 changes: 3 additions & 0 deletions src/commonMain/kotlin/com/kgit2/kommand/process/Child.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ expect class Child {
@Throws(KommandException::class)
fun wait(): Int

@Throws(KommandException::class)
fun tryWait(): Int?

@Throws(KommandException::class)
fun waitWithOutput(): Output
}
34 changes: 31 additions & 3 deletions src/commonTest/kotlin/com/kgit2/kommand/ChildTest.kt
View file
Open in desktop

@BppleMan BppleMan May 7, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think there should be a special test mechanism for tryWait, such as executing a process lasting 5 seconds through Command, and calling tryWait periodically with 1 second, and you should expect 5 nulls and 1 ok.

Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import com.kgit2.kommand.process.Command
import com.kgit2.kommand.process.Stdio
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNull

class ChildTest {
@Test
Expand All @@ -12,6 +13,7 @@ class ChildTest {
.arg("interval")
.stdout(Stdio.Pipe)
val child = command.spawn()
assertNull(child.tryWait())
val output = child.waitWithOutput()
val expect = listOf("0", "1", "2", "3", "4").joinToString("\n") + "\n"
assertEquals(expect, output.stdout)
Expand All @@ -29,6 +31,7 @@ class ChildTest {
.arg("interval")
.stdout(Stdio.Pipe)
val child = command.spawn()
assertNull(child.tryWait())
// var line = child.bufferedStdout()?.readLine()
// var expect = 0
// while (!line.isNullOrEmpty()) {
Expand All @@ -39,7 +42,25 @@ class ChildTest {
child.bufferedStdout()?.lines()?.withIndex()?.forEach {
assertEquals(it.index.toString(), it.value)
}
child.wait()
val returnCode = child.wait()
assertEquals(returnCode, child.tryWait())
assertEquals(returnCode, child.wait())
}

@Test
fun spawnTryWait() {
val command = Command(platformEchoPath())
.args("interval", "5") // waits 500 ms
.stdout(Stdio.Pipe)
val child = command.spawn()

repeat(3) {
assertNull(child.tryWait())
println(child.bufferedStdout()?.readLine())
}

val returnCode = child.wait()
assertEquals(0, returnCode)
}

@Test
Expand All @@ -49,12 +70,15 @@ class ChildTest {
.stdin(Stdio.Pipe)
.stdout(Stdio.Pipe)
val child = command.spawn()
assertNull(child.tryWait())
val expect = "Hello World!"
child.bufferedStdin()?.writeLine(expect)
child.bufferedStdin()?.flush()
val line = child.bufferedStdout()?.readLine()
assertEquals(expect, line)
child.wait()
val returnCode = child.wait()
assertEquals(returnCode, child.tryWait())
assertEquals(returnCode, child.wait())
}

@Test
Expand All @@ -68,9 +92,13 @@ class ChildTest {
for (j in 0..10000) {
child.bufferedStdin()?.writeLine("[$j]$expect")
child.bufferedStdin()?.flush()
assertNull(child.tryWait())
val line = child.bufferedStdout()?.readLine()
assertEquals("[$j]$expect", line)
}
child.wait()
val returnCode = child.wait()
assertEquals(returnCode, child.tryWait())
assertEquals(returnCode, child.wait())
assertEquals(returnCode, child.tryWait())
}
}
9 changes: 9 additions & 0 deletions src/jvmMain/kotlin/com/kgit2/kommand/process/Child.jvm.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import com.kgit2.kommand.exception.KommandException
import com.kgit2.kommand.io.BufferedReader
import com.kgit2.kommand.io.BufferedWriter
import com.kgit2.kommand.io.Output
import java.util.concurrent.TimeUnit
import java.util.concurrent.atomic.AtomicReference

actual class Child(
Expand Down Expand Up @@ -45,6 +46,14 @@ actual class Child(
return process.waitFor()
}

@Throws(KommandException::class)
actual fun tryWait(): Int? {
return when (process.waitFor(0, TimeUnit.MICROSECONDS)) {

@BppleMan BppleMan May 7, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why not use

/**
  * Returns the exit value for the process.
  *
  * @return the exit value of the process represented by this
  *         {@code Process} object.  By convention, the value
  *         {@code 0} indicates normal termination.
  * @throws IllegalThreadStateException if the process represented
  *         by this {@code Process} object has not yet terminated
  */
public abstract int exitValue();

@slomkowski slomkowski Apr 16, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

exitValue throws an exception when the process is not finished (which we should catch), waitFor seems a bit cleaner to me.

true -> wait()

@BppleMan BppleMan May 7, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same question as Child.native.kt

false -> null
}
}

@Throws(KommandException::class)
actual fun waitWithOutput(): Output {
stdin.get()?.close()
Expand Down
16 changes: 10 additions & 6 deletions src/nativeMain/kotlin/com/kgit2/kommand/Extension.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,7 @@ import kommand_core.drop_output
import kommand_core.drop_string
import kommand_core.into_output
import kommand_core.void_to_string
import kotlinx.cinterop.ByteVar
import kotlinx.cinterop.CPointer
import kotlinx.cinterop.CValue
import kotlinx.cinterop.memScoped
import kotlinx.cinterop.pointed
import kotlinx.cinterop.toKString
import kotlinx.cinterop.*

fun CPointer<ByteVar>.asString(): String {
val result = this.toKString()
Expand Down Expand Up @@ -53,6 +48,15 @@ fun Output.Companion.from(result: CValue<kommand_core.VoidResult>): Output = mem
}
}

fun Int.Companion.fromOptional(result: CValue<kommand_core.IntResult>): Int? = memScoped {
return@memScoped if (result.ptr.pointed.ok == -2) {

@BppleMan BppleMan May 7, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
return@memScoped if (result.ptr.pointed.ok == -2) {
return@memScoped if (result.ptr.pointed.error_type == /* what's for application still running */) {

// Application still running
null
} else {
Int.Companion.from(result)
}
}

@Throws(KommandException::class)
fun Int.Companion.from(result: CValue<kommand_core.IntResult>): Int = memScoped {
if (result.ptr.pointed.err != null) {
Expand Down
22 changes: 13 additions & 9 deletions src/nativeMain/kotlin/com/kgit2/kommand/process/Child.native.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,14 @@ package com.kgit2.kommand.process

import com.kgit2.kommand.exception.KommandException
import com.kgit2.kommand.from
import com.kgit2.kommand.fromOptional
import com.kgit2.kommand.io.BufferedReader
import com.kgit2.kommand.io.BufferedWriter
import com.kgit2.kommand.io.Output
import com.kgit2.kommand.io.ReaderType
import com.kgit2.kommand.unwrap
import kommand_core.buffered_stderr_child
import kommand_core.buffered_stdin_child
import kommand_core.buffered_stdout_child
import kommand_core.drop_child
import kommand_core.id_child
import kommand_core.kill_child
import kommand_core.wait_child
import kommand_core.wait_with_output_child
import kommand_core.*
import kommand_core.try_wait_child
import kotlinx.atomicfu.atomic
import kotlinx.atomicfu.locks.SynchronizedObject
import kotlinx.cinterop.COpaquePointer
Expand All @@ -23,7 +18,7 @@ import kotlin.native.ref.createCleaner

actual class Child(
private var inner: COpaquePointer?
): SynchronizedObject() {
) : SynchronizedObject() {
private var stdin: AtomicReference<BufferedWriter?> = AtomicReference(null)
private var stdout: AtomicReference<BufferedReader?> = AtomicReference(null)
private var stderr: AtomicReference<BufferedReader?> = AtomicReference(null)
Expand Down Expand Up @@ -68,6 +63,15 @@ actual class Child(
Int.from(wait_child(inner))
}

@Throws(KommandException::class)
actual fun tryWait(): Int? {
val result = Int.fromOptional(try_wait_child(inner))
return when (result) {
null -> null
else -> wait()

@BppleMan BppleMan May 7, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

According to the rust try_wait example, when an error occurs in try_wait, it should be up to the caller to choose whether to do a blocking wait, and it seems that it should not be handled by default in the library.

use std::process::Command;

let mut child = Command::new("ls").spawn().unwrap();

match child.try_wait() {
    Ok(Some(status)) => println!("exited with: {status}"),
    Ok(None) => {
        println!("status not ready yet, let's really wait");
        let res = child.wait();
        println!("result: {res:?}");
    }
    Err(e) => println!("error attempting to wait: {e}"),
}

}
}

@Throws(KommandException::class)
actual fun waitWithOutput(): Output = run {
stdin.getAndSet(null)?.close()
Expand Down