8000 safety: Mark `new_direct_byte_buffer` as `unsafe` by kjvalencik · Pull Request #320 · jni-rs/jni-rs · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

safety: Mark new_direct_byte_buffer as unsafe #320

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
Merged
merged 2 commits into from
Oct 13, 2022
Merged

safety: Mark new_direct_byte_buffer as unsafe #320

merged 2 commits into from
Oct 13, 2022

Conversation

kjvalencik
Copy link
Contributor

Overview

⚠️ Breaking Change! ⚠️

Currently, undefined behavior may be invoked with JNIEnv::new_direct_byte_buffer because it does not verify the lifetime of the received byte slice. Unfortunately, since the JVM may maintain a reference to the ByteBuffer for an unknown amount of time, it's not possible to statically enforce this.

Since we cannot lean on the borrow checker, this method is marked unsafe which is a breaking change.

An argument can be made that JNIEnv:: get_direct_buffer_address should also be marked unsafe. However, this method is safe assuming the invariants of NewDirectByteBuffer are held. It's typical practice in Rust to mark these methods safe.

Resolves #303

Definition of Done

  • There are no TODOs left in the code
  • Change is covered by automated tests
  • The coding guidelines are followed
  • Public API has documentation
  • User-visible changes are mentioned in the Changelog
  • The continuous integration build passes

@stanislav-tkach stanislav-tkach requested review from stanislav-tkach and dmitry-timofeev and removed request for stanislav-tkach June 3, 2021 09:23
stanislav-tkach
stanislav-tkach reviewed Jun 3, 2021
View reviewed changes
Copy link
Member
@stanislav-tkach stanislav-tkach left a comment

Choose a reason for hiding this comment

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

Thanks!

@rib rib mentioned this pull request Jun 7, 2022
Closed
@rib rib added this to the 0.20 milestone Oct 13, 2022
@rib rib mentioned this pull request Oct 13, 2022
Closed
6 tasks
kjvalencik and others added 2 commits October 13, 2022 21:32
@rib rib force-pushed the kv/unsafe-direct-buffers branch from 8d33c67 to 6cebc27 Compare October 13, 2022 20:35
@rib rib merged commit 6d94b9e into jni-rs:master Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stanislav-tkach stanislav-tkach stanislav-tkach approved these changes

@dmitry-timofeev dmitry-timofeev Awaiting requested review from dmitry-timofeev

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.20
Development

Successfully merging this pull request may close these issues.

Consider marking JNIEnv::new_direct_byte_buffer unsafe
3 participants
@kjvalencik @stanislav-tkach @rib
0