get_array_elements and friends unnecessarily restrict lifetimes

[jrose-signal]

Now that JNIEnv<'a> is Copy, it's clear that the lifetime of the current context is the lifetime parameter 'a, not the lifetime of any individual (Rust) reference to it. However, get_array_elements et al have an inferred result type that matches the lifetime of &self, not 'a:

pub fn get_array_elements<T: TypeArray>(
    &self,
    array: jarray,
    mode: ReleaseMode
) -> Result<AutoArray<'_, '_, T>>

This could be fixed to include 'a in the result type, but now that JNIEnv<'a> is Copy, there's no reason for AutoArray (and AutoPrimitiveArray, and JList/JMap/JavaStr) to store a reference to one at all.

pub struct AutoArray<'a, T: TypeArray> {
    obj: JObject<'a>,
    ptr: NonNull<T>,
    mode: ReleaseMode,
    is_copy: bool,
    env: JNIEnv<'a>,
}

pub fn get_array_elements<T: TypeArray>(
    &self,
    array: jarray,
    mode: ReleaseMode
) -> Result<AutoArray<'a, T>>

This would allow the AutoArray to outlast the current JNIEnv value, which makes sense when passing them around by Copy instead of by reference.

[maurolacy]

Sounds good.

Why don't you submit a PR with your suggested changes?

[rib]

As we're coming up to releasing 0.21 I wanted to give a heads up that there are changes that overlap with this change that was made for 0.20 (sorry for the churn @jrose-signal):

1. To make it harder to access invalid pointers for local references that have been deleted then object reference types like JObject or JClass etc no longer implement Copy and neither does JNIEnv : Make JObject and JNIEnv !Copy to prevent use-after-free #398
2. All the usage of jni-sys types for array references like jbyteArray now have equivalents that have an associated lifetime, like JByteArray<'local>: Add Array wrapper types with lifetimes #400

As part of #398 there is now more consistent naming and usage of a 'local lifetime that's associated with a JNIEnv<'local> that conceptually represents the current local frame. An object reference like JObject<'local> will usually take its lifetime from the environment and can't be allowed to live longer than that local frame (since that's when JNI will automatically delete those references).

As part of #400 get_array_elements takes a JPrimitiveArray (or alias such as JByteArray) and has been marked as unsafe while there are currently a few ways in which it can lead to undefined behaviour.

Hopefully these changes for 0.21 are for the better overall but it's possible they will cause some churn. Hopefully these migration notes will be of some help too: https://github.com/jni-rs/jni-rs/blob/master/docs/0.21-MIGRATION.md

[jrose-signal]

Thanks, but there are still more lifetimes than necessary. It should be possible to have a single output lifetime that is constrained by all the input lifetimes.

[rib]

Heya, just to clarify - I wasn't so much sending the heads up due to the change in lifetime parameters specifically - it was more that I was skimming over some of the 0.20 changes and happened to notice that there was a distinct overlap here with AutoArray and get_array_elements changes that have been made recently for 0.21 (mainly via #400 and #398)

Tbh I didn't remember any details of this specific change. Although I rebased and landed PR #318 for 0.20, I was probably relying on the previous review that had been done and would have generally seen that the change wasn't controversial.

It looks like the original issue here was related to inferred lifetime parameters specified via '_ that were resulting in inappropriate, automatic lifetime assignments, which shouldn't be happening anymore.

With the latest iteration of AutoArray (now AutoElements), from #400, the main aim was to make a first pass at introducing JPrimitiveArray as a reference wrapper, like JObject that could replace all the use of sys types that make it very easy to copy around and keep invalid pointers. (addressing issues #396 and #41)

As it is now, AutoElements has these associated lifetimes:

• 'local corresponds to the lifetime of the current JNI local frame.
• 'other_local is for the JNI local frame that the array reference belongs too, which may or may not be the same as the currently local frame (e.g. in case with_local_frame() is being used the array reference might be from an outer frame)
• 'array is the lifetime of the JPrimitiveArray being borrowed, to access its elements

I think those three lifetimes are currently all necessary, but maybe you can clarify what you're thinking here?

In general though I'm sure there's more that can be done to try and improve the AutoElements / get_array_elements APIs and maybe something about the lifetimes that could be improved too.

[jrose-signal]

The original issue is no longer there, yes. However, the type &'array JPrimitiveArray<'other_local, T> inside AutoElements shows the same kind of incomplete reasoning: by construction, 'other_local: 'array (that is, 'other_local must be valid at least as long as 'array is). Additionally, lifetime parameters can be shrunk as needed in all but a few cases (they are "covariant"), so you can collapse all three lifetimes in AutoElements down to one:

pub struct AutoElements<'a, T> {
    array: &'a JPrimitiveArray<'a, T>,
    len: usize,
    ptr: NonNull<T>,
    mode: ReleaseMode,
    is_copy: bool,
    env: JNIEnv<'a>,
}

impl<'a, T> AutoElements<'a, T> {
    /// # Safety
    ///
    /// `len` must be the correct length (number of elements) of the given `array`
    pub(crate) unsafe fn new_with_len<'local, 'array>(
        env: &mut JNIEnv<'local>,
        array: &'array JPrimitiveArray<'_, T>,
        len: usize,
        mode: ReleaseMode,
    ) -> Result<Self, ()>
    where
        'local: 'a,
        'array: 'a,
    {
        //...
    }
}

(You can see the compiler accepting this in the Rust playground.)

I don't know how much you want to do this kind of overhaul, but as a starting principle you almost never need multiple lifetimes in the same object. There's one smallest lifetime that's relevant, and everything else has to be alive at least that long. The main place that doesn't happen is when &mut gets involved, which is why my rewrite still uses a distinct lifetime for the env argument: we don't want to suggest that this function could mem::replace env with a new JNIEnv that has a shorter lifetime than the original.

[argv-minus-one]

@jrose-signal, here's a commit with this change applied, but what you describe doesn't seem to work for AutoElementsCritical:

    Checking jni v0.21.0-alpha.0
error[E0621]: explicit lifetime required in the type of `env`
  --> src/wrapper/objects/auto_elements_critical.rs:56:9
   |
34 |           env: &'env mut JNIEnv,
   |                ---------------- help: add explicit lifetime `'auto_elem` to the type of `env`: `&'env mut jnienv::JNIEnv<'auto_elem>`
...
56 | /         Ok(AutoElementsCritical {
57 | |             array,
58 | |             len,
59 | |             ptr: NonNull::new(ptr).ok_or(Error::NullPtr("Non-null ptr expected"))?,
...  |
62 | |             env,
63 | |         })
   | |__________^ lifetime `'auto_elem` required

error[E0621]: explicit lifetime required in the type of `env`
  --> src/wrapper/objects/auto_elements_critical.rs:57:13
   |
34 |         env: &'env mut JNIEnv,
   |              ---------------- help: add explicit lifetime `'array` to the type of `env`: `&'env mut jnienv::JNIEnv<'array>`
...
57 |             array,
   |             ^^^^^ lifetime `'array` required

error: lifetime may not live long enough
  --> src/wrapper/objects/auto_elements_critical.rs:57:13
   |
34 |         env: &'env mut JNIEnv,
   |         --- has type `&mut jnienv::JNIEnv<'2>`
35 |         array: &'array JPrimitiveArray<T>,
   |         ----- has type `&jprimitive_array::JPrimitiveArray<'1, T>`
...
57 |             array,
   |             ^^^^^ this usage requires that `'1` must outlive `'2`

error[E0621]: explicit lifetime required in the type of `env`
  --> src/wrapper/objects/auto_elements_critical.rs:62:13
   |
34 |         env: &'env mut JNIEnv,
   |              ---------------- help: add explicit lifetime `'env` to the type of `env`: `&'env mut jnienv::JNIEnv<'env>`
...
62 |             env,
   |             ^^^ lifetime `'env` required

For more information about this error, try `rustc --explain E0621`.
error: could not compile `jni` due to 4 previous errors

But if I add the 'local lifetime parameter back, it works. Why is that?

[jrose-signal]

It goes to this part:

The main place that doesn't happen is when &mut gets involved, which is why my rewrite still uses a distinct lifetime for the env argument: we don't want to suggest that this function could mem::replace env with a new JNIEnv that has a shorter lifetime than the original.

For the non-Critical version, the lifetime of the reference to JNIEnv wasn't important for the resulting AutoElements, because the function did an unsafe_clone of the environment and "released" its hold on the original JNIEnv. But the Critical one needs to hold it, and so it now contains an &mut T where T itself contains a reference (JNIEnv<'local>). Because &mut allows wholesale replacement, its argument is "invariant", and thus the lifetime inside the JNIEnv cannot be shortened to match the lifetime of the &mut. Otherwise I would be able to do this:

func bad<'ref, 'local>(env: &'ref mut JNIEnv<'local>) {
  let completely_new_jvm = spin_up_new_jvm();
  let new_env: JNIEnv<'short> = completely_new_jvm.env(); // you can't actually name 'short in Rust, but pretend you can
  let short_env: &'short mut JNIEnv<'local> = env; // this is okay, 'short is smaller than 'ref
  let short_env_only: &'short mut JNIEnv<'short> = short_env; // this is not okay, because of the next line
  *short_env_only = new_env; // oops!
}

So if you're wrapping something that's &mut, like AutoElementsCritical, you'll need to propagate through any lifetimes inside the &mut, plus you have the one top-level lifetime that's "the lifetime where you can use this AutoElementsCritical".

[rib]

One thing that probably also needs to be considered here is that .with_local_frame_returning_local() currently intentionally creates a new lifetime name for the local frame which isn't related to / constrained by the outer 'local lifetime.

(so I think that can give us separate 'local / 'other_local reference that are invariant)

with_local_frame_returning_local() takes a closure that lets you return a local reference to the outer scope. The way that we create a 'new_local name for the closure was intended to stop code from moving a local reference from the outer scope into the closure and then return it back again to the outer scope.

Originally I had thought that would actually be undefined JNI behaviour to try and do that but it looks like that's probably actually OK to do as far as JNI is concerned, but it does results in a duplicate reference where you're not able to explicitly drop the original reference.

See discussion here, for more context: #392 (comment)

We could re-consider whether that's really a problem and potentially change this, but wanted to highlight that here since I think it may break some of the assumptions above.

[jrose-signal]

It does not. What makes the 'new_local pattern safe here is that it's for<'new_local>; that is, the closure has to be able to handle any lifetime here, whether shorter than, longer than, or the same as 'local. Because of that, you can't turn a JObject<'local> into a JObject<'new_local>, because, weirdly, 'local does not live long enough. If you add the constraint where 'local: 'new_local, you'll see that you're permitted to turn a JObject<'local> into JObject<'new_local> but not the reverse.

[jrose-signal]

At the same time, you have a number of 'other_local arguments that are unnecessary, such as

pub fn alloc_object<'other_local, T>(
    &mut self,
    class: T
) -> Result<JObject<'local>>
where
    T: Desc<'local, JClass<'other_local>>

Why isn't this just T: Desc<'local, JClass<'local>>? T only needs to be able to describe classes in the current environment, not any other one the caller chooses. You do still need the outer lifetime, since the inner one is inside Desc's generic argument, but they shouldn't be different lifetimes here.

[rib]

At least for me I've been thinking that 'other_local vs 'local was necessary because of my (I think now, probably incorrect) mental model that with_local_frame_returning_local could create disconnected (invariant) lifetimes for local frames.

So in this case I thought the seperation was necessary to ensure it would be possible to allocate an object based on a class that exists outside the current with_local_frame.. closure, with code something like:

fn foo(env: &mut JNIEnv) -> Result<()> {
    let outer_klass = env.find_class(FOO_CLASS)?;

    let ref = env.with_local_frame_returning_local(|env| {
        let obj = env.alloc_object(outer_klass);
        Some(obj)
    })?;
    Ok(())
}

[jrose-signal]

I guess you can't have it both ways. Either the local frame prevents references from outside the local frame, or it doesn't; as noted elsewhere, having a second JNIEnv on the same thread breaks many of these assumptions.

[jrose-signal]

Alternately, you can force all these lifetimes to be invariant, by using PhantomData<*mut &'local ()> instead of just PhantomData<&'local ()>. But now we're moving beyond my expertise too; while it seems like an interesting property, I don't actually know how well the compiler deals with it in practice.

EDIT: here's a slightly more polished version from compact_arena.

[rib]

not quite sure which "assumptions" you're referring to from having multiple env's per thread but something else to perhaps note is that any API that can create new references now has to mutably borrow the environment, and since .with_local_frame* mutably borrow the env then that limits your ability to continue using a JNIEnv from an outer frame.

[rib]

I think the only reason (at least in my mind) for wanting them to be invariant was to account for the weird case where code tries to move a local reference from an outer scope into a with_local_frame_returning_local closure and then return that back.

Personally I would be happy to remove with_local_frame_returning_local since I think the special case optimization for returning local references isn't that compelling (benchmarking returning a GlobalRef wasn't much slower).

Otherwise I wonder if it's actually reasonable enough to just document and warn against this corner case with with_local_frame_returning_local instead of trying to support invariant local frame lifetimes.

[argv-minus-one]

[@jrose-signal] If you add the constraint where 'local: 'new_local

You mean add that constraint to JNIEnv::with_local_frame{,_returning_local}? Where would that constraint go? This isn't syntactically valid:

pub fn with_local_frame<F, T, E>(&mut self, capacity: i32, f: F) -> std::result::Result<T, E>
where
    F: for<'new_local where 'local: 'new_local> FnOnce(&mut JNIEnv<'new_local>) -> std::result::Result<T, E>,
    E: From<Error>,
{}

As far as I know, an HRTB lifetime can be declared to outlive some other lifetime, but not the reverse, and we need the reverse here.

If we can do this somehow, it should be sound to do so, since this should allow local references' lifetimes to be narrowed but not widened. (Correct me if I'm wrong!)

[@jrose-signal] Why isn't this just T: Desc<'local, JClass<'local>>? T only needs to be able to describe classes in the current environment, not any other one the caller chooses.

It does need to describe JClasses from other local reference frames, so that this will work:

let mut outer_env: JNIEnv<'outer_local> = _;

let some_class: JClass<'outer_local> = _;

outer_env.with_local_frame(|inner_env: JNIEnv<'inner_local>| {
	inner_env.call_static_method(&some_class, "someMethod", "()V", &[])?.v()
})?;

The JClass here doesn't have to have the same 'local as the JNIEnv that it's being used with. It just has to still be alive. This reflects how C JNI works: you can freely use a local reference from any frame at any time as long as that frame belongs to the current thread and is still open. It is, for example, valid to create a local reference in one frame and delete it in another.

[@rib] I think the only reason (at least in my mind) for wanting them to be invariant was to account for the weird case where code tries to move a local reference from an outer scope into a with_local_frame_returning_local closure and then return that back.

Why? As far as I know, that's perfectly valid. It works, -Xcheck:jni doesn't complain, and the JNI spec doesn't say the returned reference has to belong to the frame being popped.

[argv-minus-one]

Reopening this since we seem to still be discussing it.

[rib]

Why? As far as I know, that's perfectly valid. It works, -Xcheck:jni doesn't complain, and the JNI spec doesn't say the returned reference has to belong to the frame being popped.

Yep, you confirmed it works before, it just results in an extra reference and you can no longer explicitly drop the original reference. (ref your comment here too: #392 (comment))

(so even though we determined that JNI is fine with this we briefly noted at the time that it could still lead to an awkward leak)

[argv-minus-one]

Ah, I thought I had looked into this before. Thanks for the reminder.

@jrose-signal, how do you feel about this? If we connect the outer and inner lifetimes, then we can eliminate some potentially-confusing and arguably-unnecessary lifetime parameters from JNIEnv methods like alloc_object, but we also create a footgun for memory leaks as discussed in the comment @rib just linked.

And, again, if we are to connect these two lifetimes, then I also need to know how to do that with HRTB syntax.

[rib]

@argv-minus-one considering your example about accessing a class from an outer frame is almost the same as the one I gave (#302 (comment)), I guess we may both have had a similar misunderstanding here.

Actually if I make this change:

index 12c5c71..6b0c34f 100644
--- a/src/wrapper/jnienv.rs
+++ b/src/wrapper/jnienv.rs
@@ -895,9 +895,9 @@ impl<'local> JNIEnv<'local> {

     /// Allocates a new object from a class descriptor without running a
     /// constructor.
-    pub fn alloc_object<'other_local, T>(&mut self, class: T) -> Result<JObject<'local>>
+    pub fn alloc_object<T>(&mut self, class: T) -> Result<JObject<'local>>
     where
-        T: Desc<'local, JClass<'other_local>>,
+        T: Desc<'local, JClass<'local>>,
     {
         let class = class.lookup(self)?;
         let obj = jni_non_null_call!(self.internal, AllocObject, class.as_ref().as_raw());

this code actually builds and runs fine:

#[test]
fn with_local_frame_access_outer_ref() {
    let mut env = attach_current_thread();
    let int_class = env.find_class(INTEGER_CLASS).unwrap();

    let _obj = env.with_local_frame_returning_local(16, |env| {
        env.alloc_object(int_class)
    }).unwrap();
}

(though I would have previously not expected that to work, because I thought that the two environment lifetimes were more disconnected than they are)

[jrose-signal]

You mean add that constraint to JNIEnv::with_local_frame{,_returning_local}? Where would that constraint go? This isn't syntactically valid:

You're correct. I thought you could put it after the function result, like a top-level where clause, but you can't, and I'm not sure if there's a standard way to deal with this. The (few) discussions I'm seeing around this suggest that you'd have to make an intermediate trait, and that's pretty unfortunate.

(I do also think calling this a "memory leak" is incorrect; the memory is held on to longer than necessary, but it will still be released when the outer frame goes away.)

[rib]

I do also think calling this a "memory leak" is incorrect; the memory is held on to longer than necessary, but it will still be released when the outer frame goes away.

NB: If your JNI code is running in a thread that's not part of a native method implementation then you may have a base local frame that you don't unwind.

Still though it's quite a contrived set of circumstances. It's very hard to imagine this corner case being hit in a way that's really problematic.

Arguably it's just a programming error. With the way that references are non-Copy now there can be plenty of ways in which you could lose access to a reference by moving / dropping a JObject and the underlying reference is only deleted when the frame unwinds.

If you have code that is going to run within a thread where the frame won't unwind regularly then it's probably fair to say that you need to be conscious of cleaning up any transient local references.

[argv-minus-one]

This, however, doesn't work:

fn with_local_frame_access_outer_ref<'local>(
    env: &mut JNIEnv<'local>,
    int_class: &JClass,
) -> Result<JObject<'local>> {
    env.alloc_object(int_class)
}

error[E0621]: explicit lifetime required in the type of `int_class`
   --> tests/jni_api.rs:245:5
    |
243 |     int_class: &JClass,
    |                ------- help: add explicit lifetime `'local` to the type of `int_class`: `&JClass<'local>`
244 | ) -> Result<JObject<'local>> {
245 |     env.alloc_object(int_class)
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime `'local` required

The compiler isn't happy unless env and int_class are given connected lifetimes:

fn with_local_frame_access_outer_ref<'local>(
    env: &mut JNIEnv<'local>,
    int_class: &JClass<'local>,
) -> Result<JObject<'local>> {
    env.alloc_object(int_class)
}

Seems like an unnecessary and unergonomic restriction.

[jrose-signal]

Are you sure that JClass<'outer> isn't being converted to JClass<'inner> even though the lifetimes are supposed to be unrelated? I wouldn't expect that to happen, but we've already broken my expectations at least once.

[argv-minus-one]

I'm not sure what you mean by 'outer and 'inner. There are no lifetimes with those names in this snippet.

If you're referring to local reference frames, this function doesn't actually create a new local reference frame (sorry, I forgot to rename it). It's just a function that accepts a &mut JNIEnv and a &JClass and allocates an object of that class. This function doesn't care which local reference frame the JClass belongs to, as long as the JClass is still valid.

[jrose-signal]

Sorry, I'm mixing code snippets to respond to. I was wondering more why the previous example worked than why this one didn't, and my conclusion is that there are bunch of complicated Desc implementations such that I can't manually trace where the lifetime is changing. ><

Stepping back, I see that adding lifetimes to everything makes sense: you don't want locals to outlive a JNIEnv. You're also trying to have nested local frames use a different lifetime, but those nested frames can still use values attached to the lifetime of the outer frame. You're not trying to protect against multiple independent JNIEnvs being in play at the same time using lifetimes, which is good because lifetimes aren't great at that (being covariant by default). I did not understand all that when I started my discussion.

Nevertheless, I'm still pretty sure AutoElements only needs one lifetime parameter (the minimum lifetime of all its local references), and AutoElementsCritical only needs two (the minimum of all its local references, plus the exact lifetime attached to the JNIEnv it holds). Same for JList and JMap and JValue (the referency one).

[rib]

Another thing to note here that seems relevant (considering &mut T implies invariance) is that #400 had initially updated get_array_elements[_critical]() so they mutably borrowed the JPrimitiveArray that they were accessing.

The reason for this was to help ensure you can't easily create multiple AutoElements that alias (due to the possibility of undefined behavior if you create multiple mutable references to the elements via DerefMut). It would also help avoid re-entrancy on the Rust side if you were relying on a monitor to synchronize access to an array. (can see some context here: #400 (comment))

This got reverted at the last minute though due to a concern about the ergonomics we'd have with GlobalRef arrays but ideally I think that should be put back at some point.

In terms of consolidating these lifetimes, I'm honestly a little bit confused currently.

As noted - my mental model so far has been that we needed to consider invariant JNIEnv 'local lifetimes coming from with_local_frame_; where we would want to allow code to read local references from the current 'local frame as well as an (invariant) 'outer_local frame.

I don't currently have a clear understanding of how the HRTB 'new_local lifetime theoretically relates to the input 'local time, and it also looks like we can't currently add an explicit constraint that would connect 'local and 'new_local how we'd want?

I might have misunderstood this comment: #302 (comment) that I initially thought was asserting that 'local and 'new_local are not invariant - and I might have added to my confusion with this experiment: #302 (comment) that apparently confirmed for me that they aren't invariant - but I'm not confident about that conclusion. Since @jrose-signal was also suggesting adding a where 'local: 'new_local constraint (which we've since seen isn't possible) does that mean that you did actually expect that they would be invariant lifetimes currently (without adding a constraint)? With the indirection of the Desc trait it's also not super clear what should be concluded from the experiment I did.

Summary

I guess (but am not confident) that we actually do need to consider invariant 'local frame lifetimes currently, from with_local_frame_...

We'd probably be happy to have covariance via a 'local: 'new_local constraint for the with_local_frame_ closures if that's possible (since I think we could agree that the corner case of sending back 'outer_local references can be considered a programmer error) but it sounds like this isn't currently possible?

If we're stuck with needing to consider invariant 'local frame lifetimes we need to keep the 'local and other_local lifetimes for AutoElements.

Although the 'array lifetime should be covariant with 'other_local, if we want to be able to turn this into a mutable reference later I think we may also need to keep the separate lifetime for that too?

So maybe for now we do need to keep these three lifetimes?

[rib]

This, however, doesn't work:

fn with_local_frame_access_outer_ref<'local>(
    env: &mut JNIEnv<'local>,
    int_class: &JClass,
) -> Result<JObject<'local>> {
    env.alloc_object(int_class)
}

error[E0621]: explicit lifetime required in the type of `int_class`
   --> tests/jni_api.rs:245:5
    |
243 |     int_class: &JClass,
    |                ------- help: add explicit lifetime `'local` to the type of `int_class`: `&JClass<'local>`
244 | ) -> Result<JObject<'local>> {
245 |     env.alloc_object(int_class)
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime `'local` required

The compiler isn't happy unless env and int_class are given connected lifetimes:

fn with_local_frame_access_outer_ref<'local>(
    env: &mut JNIEnv<'local>,
    int_class: &JClass<'local>,
) -> Result<JObject<'local>> {
    env.alloc_object(int_class)
}

Seems like an unnecessary and unergonomic restriction.

yeah this is a tricky one.

I've been thinking it feels a bit lucky that we do get away with not needing to explicitly connect the lifetimes of native method arguments like this but at the same time it's convenient that it's not currently required.

we can see this as a simpler reason (than talking about with_local_frame_) for why we always need to be considerate of invariant 'local lifetimes.

conceptually it's a bit like all of these arguments belong to a series of separate outer_local_1, outer_local_2 frames.

[argv-minus-one]

From the label you've added, I take it we're going to leave this alone for now and think about it later?

[rib]

Yeah, I think for now at least it seems like it's reasonable to not block the 0.21 release based on this.

In a number of places (not just for get_array_elements we have generally assumed we may sometimes be dealing with more than one JNI local frame and we assume they may have invariant / unrelated lifetimes

Maybe we can change this by investigating what constraints we can convey via HRTBs / with_local_frame_ and also re-consider how to handle native method references that don't explicitly connect their lifetime to the JNIEnv - but this seems like quite a high-level design issue that I'd like to defer until after 0.21 ideally.

We can maybe assume that the 'array lifetime is contained by the 'other_local and remove one lifetime now if we feel strongly about that?

In general though I'd quite like to make a 0.21 release soon and not get too hung up on this.

I feel like this API isn't exactly perfect in a number of ways, so I also wouldn't be surprised if we end up making this issue redundant anyway when thinking about how to tackle other safety issues with the current API.

I don't want to cause unnecessary churn for people but at the same time, some corners of the jni API (including the AutoArray/AutoElements API) do have quite notable issues with them and so I'd also like to see some incremental progress with e.g. improving safety. Overall I think 0.21 has some good changes in it, even if we know there are things that need further work.

Does that sound reasonable @argv-minus-one + @jrose-signal ?

[jrose-signal]

Keeping the issue open seems like a reasonable compromise for now. And at least 0.21 shouldn't have the original issue of tying the lifetime to the JNIEnv reference instead of the array object, if I'm reading it correctly.

[rib]

Okey, thanks for the ACK. I think that means I can probably make the 0.21 release either tonight or tomorrow 🤞