Skip to content

[Android] Don't use attach_current_thread_permanently, leave that decision up to the client #184

Description

@jrose-signal

The Android implementation works without additional context by storing the JVM in a global and attaching to it as needed. However, it uses attach_current_thread_permanently to do so.

fn env(&self) -> Result<JNIEnv, Error> {
let vm = match self {
Global::Internal { java_vm, .. } => java_vm,
Global::External(global) => global.java_vm(),
};
Ok(vm.attach_current_thread_permanently()?)
}

This can affect and be affected by any other uses of the JVM from this thread:

  • If the thread was originally a Java thread, basically nothing happens.
  • If the thread was already attached manually, basically nothing happens.
  • If the thread was already attached as a daemon, nothing happens, but it's unclear to me whether the JVM can still shut down while the thread is active but also considered a daemon.
  • If the thread was not previously attached, it's now permanently attached, blocking shutdown, and cannot later be detached or marked as a daemon

rustls-platform-verifier should instead use the plain old attach_current_thread, and recommend to clients to call attach_current_thread_permanently for less overhead.

(We ran into this, or think we did, because we used rustls-platform-verifier from a thread pool that also attached using plain old attach_current_thread elsewhere, and our code was relying on detaching freeing local references. We will probably change that to be explicit anyway, but still, rustls-platform-verifier shouldn't be making the decision.)

Metadata

Metadata

Assignees

Labels

O-AndroidWork related to the Android verifier implementationbugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions