Skip to content

coredump in TryCopyLastError #759

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

Jump to bottom
Open
hnwyllmm opened this issue May 20, 2025 · 2 comments · May be fixed by #760
Open

coredump in TryCopyLastError #759

hnwyllmm opened this issue May 20, 2025 · 2 comments · May be fixed by #760
Labels
help wanted Extra attention is needed Type: bug Something isn't working

Comments

@hnwyllmm
Copy link
Contributor

hnwyllmm commented May 20, 2025 •
edited
Loading

Describe the bug, including details regarding any error messages, version, and platform.

If Consumer throws exception, TryCopyLastError would copy exception message to C buffer. TryCopyLastError assume the string(byte[]) returned by lastError is '\0' terminated and test the length of the string by strlen which result in invalid memory access. In actually, lastError is a UTF8 encoded buffer(byte[]).

The code of TryCopyLastError:

  jobject error_data =
      env->GetObjectField(private_data->j_private_data_, kPrivateDataLastErrorField);
  ...
  auto arr = reinterpret_cast<jbyteArray>(error_data);
  jbyte* error_bytes = env->GetByteArrayElements(arr, nullptr);
  if (!error_bytes) {
    private_data->last_error_.clear();
    return;
  }

  char* error_str = reinterpret_cast<char*>(error_bytes);
  private_data->last_error_ = std::string(error_str, std::strlen(error_str));

The code below shows how lastError be setted:

private int setLastError(Throwable err) {
      // Do not let exceptions propagate up to JNI
      try {
        StringWriter buf = new StringWriter();
        PrintWriter writer = new PrintWriter(buf);
        err.printStackTrace(writer);
        lastError = buf.toString().getBytes(StandardCharsets.UTF_8);
      } catch (Throwable e) {
        // Bail out of setting the error message - we'll still return an error code
        lastError = null;
      }
      return 5; // = EIO
    }
@hnwyllmm hnwyllmm added the Type: bug Something isn't working label May 20, 2025
@hnwyllmm
Copy link
Contributor Author

I'd like to fix this issue.

@lidavidm
Copy link
Member

PRs are welcome!

@lidavidm lidavidm added the help wanted Extra attention is needed label May 20, 2025
@hnwyllmm hnwyllmm linked a pull request May 20, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted Extra attention is needed Type: bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

GH-759: Get length of byte[] in TryCopyLastError hnwyllmm/arrow-java
2 participants
@lidavidm @hnwyllmm