[Pengkun-ZHU]

Description

A push to a closed mutable-object channel can return an expected ChannelError from the object manager. The provider previously treated errors from WriteAcquire and WriteRelease as fatal with RAY_CHECK_OK, crashing the receiving Raylet.

This fix returns Status from HandlePushMutableObject and propagates it through the Raylet RPC response:

• On the receiver side, clear partial chunk-progress state upon failure so a failed transfer does not leave stale accounting behind.
• On the sender, invoke the logical push callback for RPC failures as well as completed replies, with an at-most-once guard across the chunks of a push.
• Track push results per remote reader. Close the writer channel on the first reported failure without waiting for the remaining readers. Resume polling only after all readers complete successfully. Log failures to close the channel.

Added regression tests for pushing to a closed reader channel, closing a writer channel before other readers reply, and continuing to poll after successful pushes.

N.B. This is fail-fast handling, not recovery. A transient transport failure also closes the writer channel; there are no retries, timeouts, or recovery protocol. If an RPC never returns and no other reader reports a failure, the transfer may still remain pending indefinitely.

Related issues

Fixes #63229

Additional information

[gemini-code-assist]

Code Review

This pull request improves error handling and status propagation for mutable object pushes across nodes. Key changes include updating HandlePushMutableObject to return a Status, introducing thread-safe tracking of remote push completions and failures, and ensuring that failures close the corresponding channels. Additionally, tests were added to verify these failure paths. The review feedback highlights a critical correctness bug in raylet_client.cc where a chunk size of zero can cause a hang when data_size is a multiple of kMaxGrpcPayloadSize, and suggests an optimization in experimental_mutable_object_provider.cc to fail fast and close the writer channel on the first remote reader failure.

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Reviewed by Cursor Bugbot for commit 1f46331. Configure here.}

[Pengkun-ZHU]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves error handling and status propagation in the mutable object provider and Raylet client. It updates HandlePushMutableObject to return a Status and clean up state on failure, and modifies the writer polling logic and RPC client to handle remote push failures thread-safely. Unit tests have also been added to cover these failure paths. Feedback on the changes highlights a critical security vulnerability where incoming request parameters (offset, chunk_size, and total_metadata_size) are not validated against actual payload sizes, potentially leading to out-of-bounds writes or reads.

[gemini-code-assist]

Security Vulnerability (Buffer Overflow / Over-read): The incoming PushMutableObjectRequest parameters (offset, chunk_size, and total_metadata_size) are not validated against the actual sizes of the data and metadata payloads, nor against the total allocated data size. This can lead to out-of-bounds writes (buffer overflow) on the receiver side during memcpy operations, or out-of-bounds reads (buffer over-read) if the payload sizes are smaller than expected.

Please validate these bounds before performing any operations or allocating resources.

  if (offset + chunk_size > total_data_size ||
      tmp_written_so_far + chunk_size > total_data_size ||
      request.data().size() < chunk_size ||
      request.metadata().size() < total_metadata_size) {
    clear_written_so_far();
    return Status::InvalidArgument("Invalid chunk offset, size, accumulated length, or metadata size");
  }
  if (tmp_written_so_far == 0u) {

[Pengkun-ZHU]

I checked upstream code and found this appears to be a real issue.
The size check in MutableObjectManager::WriteAcquire only verifies that the declared data-plus-metadata size fits the receiver’s buffer. It does not verify that each chunk’s offset and length stay within that buffer, or that its payload contains the claimed number of bytes.
For example, an 80-byte declared total fits a 100-byte buffer, but copying 10 bytes at offset 95 still writes past its end.

As it is related to existing code that predates this very PR, I'll leave it for committers to comment.