Conversation
bc95b08 to
ef2519b
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces LibUV integration and basic async primitives to Koka, enabling asynchronous I/O operations with timer support. The implementation supports both native platforms (via LibUV) and WebAssembly (via Emscripten).
Key changes include:
- Complete async effect system with promises, channels, timeouts, and interleaved execution
- LibUV timer integration with both native and WASM implementations
- Event loop management with automatic allocator configuration
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Compile/Options.hs | Adds WASM_BIGINT emscripten linker flag |
| src/Compile/CodeGen.hs | Extends C code generation to include external imports from all imported modules |
| lib/std/async/async.kk | Core async effect system with promises, channels, timeouts, and interleaving |
| lib/std/async/null.kk | Null type conversions for external function interfaces |
| lib/std/async/inline/timer.js | JavaScript timer implementation (duplicate of uv/inline/timer.js) |
| lib/std/async.kk | Public API re-export module |
| lib/uv/utils.kk | UV status codes and error handling utilities |
| lib/uv/timer.kk | Timer API with duration-based interface |
| lib/uv/inline/utils.h | C macros for handle management and error checking |
| lib/uv/inline/utils.c | UV status code to Koka enum conversion |
| lib/uv/inline/timer.h | Timer handle structure definitions for UV and WASM |
| lib/uv/inline/timer.c | Timer implementation for both UV and WASM platforms |
| lib/uv/inline/timer.js | JavaScript timer primitives |
| lib/uv/inline/event-loop.c | Event loop initialization and execution |
| lib/uv/event-loop.kk | High-level event loop API and default async handler |
| samples/async/timer.kk | Example demonstrating timer usage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| extern import | ||
| c { conan="libuv[>=1.47.0]"; vcpkg="libuv"; library="uv" } |
There was a problem hiding this comment.
The extern import for libuv pins the third-party dependency using a mutable version range (conan="libuv[>=1.47.0]", vcpkg="libuv"), so builds may transparently pull in new, unreviewed libuv releases with full access to your process and secrets. This creates a supply-chain risk where a compromised libuv release or upstream repository could execute arbitrary code during build or at runtime. Prefer pinning to immutable identifiers (exact version or commit/digest) and/or enforcing integrity verification for fetched artifacts to limit the blast radius of an upstream compromise.
| extern import | ||
| c { conan="libuv[>=1.47.0]"; vcpkg="libuv"; library="uv" } |
There was a problem hiding this comment.
The extern import for libuv here again uses mutable version selection (conan="libuv[>=1.47.0]", vcpkg="libuv"), allowing your toolchain to auto-upgrade to new libuv releases that have not been explicitly vetted. Because this code runs inside your process and event loop, a malicious or compromised upstream libuv version can lead to arbitrary code execution or data exfiltration via the build or runtime environment. Pin this dependency to an immutable version/commit (and use checksums or signatures where possible) to reduce supply-chain attack exposure.
cb720fd to
18c0db1
Compare
| @@ -0,0 +1,38 @@ | |||
| // Used for external interfaces | |||
| module std/async/null | |||
There was a problem hiding this comment.
Not async specific, maybe this belongs in something like std/ffi?
There was a problem hiding this comment.
Good question. I'll let Daan decide the namespace for this.
18c0db1 to
9e28d76
Compare
| static void kk_uv_loop_close(kk_context_t* _ctx) { | ||
| int ret = uv_loop_close(uvloop()); | ||
| if (ret != 0) { | ||
| kk_warning_message("Event loop closed %s\n", uv_err_name(ret)); |
There was a problem hiding this comment.
IMO this should be an error that we propagate, and if it reaches the default handler then that can either throw (if allowed) or exit with nonzero status.
Usually a failure in uv_loop_close indicates an improperly cleaned up handle, which is worth knowing about.
There was a problem hiding this comment.
Yeah, I considered making it just exit immediately actually. I'm not sure what the purpose of letting the program handle this would be, since it almost surely means that there is a serious error in the program or in our libUV bindings. Ideally we would enforce that all handles are cleaned up (references don't escape the scope of the loop) via a scoped type parameter like the local or state effect st<h>. I'm not sure that is the best idea though, locals are typically locally guaranteed to not escape, and st<h> often is used with global or essentially for pretty local state. Having such a type parameter go throughout the program might be unwieldy.
Maybe a compromise would be to add a scoped type parameter, and then expose the resources (or just apis to access parts of the resources) to the rest of the code via effect handlers. Maybe named handlers actually. I'm curious how timers, etc would look with this design.
5e7c518 to
c88f651
Compare
* uv timer: reduce API surface and simplify callback management * cleanup uv/utils * add uv-status-code/show impl * report open handle counts and types when event-loop close reports UV_EBUSY * fix new naming * fix allocation now that real fix to koka-lang#864 landed --------- Co-authored-by: Tim Cuthbertson <tim@gfxmonk.net>
Here is a more simple PR, without files, streams, etc, and cleaned up and documented based on things I've learned since I implemented it last time.
This PR contributes a working async event loop + timers on C/JS/WASM. I've tested with a leak checker on MacOS to ensure no memory leaks.