chore: 修复库路径#1
Conversation
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change updates CI aggregation, removes Composer extension metadata, redirects prebuilt binary downloads, and reformats PHP and Rust implementation code while preserving the described runtime behavior. ChangesRelease and runtime updates
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
rust/src/runtime.rs (1)
334-342: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHandle errors from
try_waitto prevent zombie tracking.If
child.try_wait()returns anErr(e)(e.g., the child was reaped externally or an OS error occurred), the current wildcard_arm pushes it back intokept. This can lead to an infinite loop of attempting to wait on a non-existent child process in future cycles without successfully restarting it.Consider matching
Ok(None)explicitly to keep the running child, and treatingErrsimilarly toOk(Some(_))(by restarting it) to ensure resilience.🛠️ Proposed fix
- match child.try_wait() { - Ok(Some(_)) => { - to_restart.push(tracked.kind); - } - _ => { - kept.push(TrackedChild { - kind: tracked.kind, - child, - }); - } - } + match child.try_wait() { + Ok(Some(_)) => { + to_restart.push(tracked.kind); + } + Ok(None) => { + kept.push(TrackedChild { + kind: tracked.kind, + child, + }); + } + Err(e) => { + eprintln!("[lychee-worker] failed to wait on child (pid={}): {}", child.id(), e); + to_restart.push(tracked.kind); + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/src/runtime.rs` around lines 334 - 342, Update the child handling match around child.try_wait() to distinguish Ok(None) from errors: keep the child only for Ok(None), while treating Err cases like Ok(Some(_)) by adding tracked.kind to to_restart and not retaining the child in kept.rust/src/http.rs (1)
93-96: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPrevent CPU spin on
accepterrors (e.g.,EMFILE).If the process runs out of available file descriptors,
listener.accept()will immediately and repeatedly return an error (such asEMFILEorENFILE). Without a delay, this causes the accept loop to spin instantly, pegging the CPU at 100% and flooding the logs.Consider introducing a short sleep on error to allow the system time to recover before attempting to accept new connections.
🛠 Proposed fix
Err(e) => { eprintln!("[lychee-worker] accept error: {}", e); + tokio::time::sleep(std::time::Duration::from_millis(100)).await; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/src/http.rs` around lines 93 - 96, Update the accept error branch in the listener loop to sleep briefly after logging an error, before retrying listener.accept(). Reuse the module’s existing async/runtime sleep mechanism if available, and preserve the current retry behavior for transient errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/src/websocket.rs`:
- Around line 100-101: Update the connection task around the reader and writer
JoinHandles to declare both handles mutable and use tokio::select! instead of
tokio::join!. When either reader or writer completes, explicitly abort the other
handle before returning so cleanup_connection runs and releases the connection
resources.
---
Nitpick comments:
In `@rust/src/http.rs`:
- Around line 93-96: Update the accept error branch in the listener loop to
sleep briefly after logging an error, before retrying listener.accept(). Reuse
the module’s existing async/runtime sleep mechanism if available, and preserve
the current retry behavior for transient errors.
In `@rust/src/runtime.rs`:
- Around line 334-342: Update the child handling match around child.try_wait()
to distinguish Ok(None) from errors: keep the child only for Ok(None), while
treating Err cases like Ok(Some(_)) by adding tracked.kind to to_restart and not
retaining the child in kept.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 398422b9-99fe-4ada-ab65-3086f9658c3b
📒 Files selected for processing (7)
build.rsrust/src/http.rsrust/src/lib.rsrust/src/php_api.rsrust/src/runtime.rsrust/src/stats.rsrust/src/websocket.rs
| let _ = tokio::join!(reader, writer); | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Task deadlock and resource leak on client disconnect.
tokio::join!(reader, writer) waits for both spawned tasks to complete. If the client disconnects, the reader task finishes, but the writer task remains blocked indefinitely on rx.recv().await. Because the join! never returns, cleanup_connection is never called, meaning tx is never removed from the CONNECTIONS map and rx never closes. This permanently leaks the task and connection resources for every disconnected client.
Use tokio::select! so that when one task completes, the other is aborted. Note that dropping a Tokio JoinHandle does not automatically cancel the background task, so you must explicitly call .abort().
🛠 Proposed fix
Change the reader and writer variables to be mutable, and replace tokio::join! with tokio::select!:
- let writer = tokio::task::spawn_local(async move {
+ let mut writer = tokio::task::spawn_local(async move {
// ...
- let reader = tokio::task::spawn_local(async move {
+ let mut reader = tokio::task::spawn_local(async move {
// ...
- let _ = tokio::join!(reader, writer);
+ tokio::select! {
+ _ = &mut reader => { writer.abort(); }
+ _ = &mut writer => { reader.abort(); }
+ }📝 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.
| let _ = tokio::join!(reader, writer); | |
| }) | |
| tokio::select! { | |
| _ = &mut reader => { | |
| writer.abort(); | |
| } | |
| _ = &mut writer => { | |
| reader.abort(); | |
| } | |
| } | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust/src/websocket.rs` around lines 100 - 101, Update the connection task
around the reader and writer JoinHandles to declare both handles mutable and use
tokio::select! instead of tokio::join!. When either reader or writer completes,
explicitly abort the other handle before returning so cleanup_connection runs
and releases the connection resources.
1 participant
Summary by CodeRabbit
all-checksCI job that aggregates required checks and reports completion.