Skip to content
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.

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

[jni] Deadlock when calling Interceptor.Chain.proceed() via bindings of Kotlin Package: OkHttp #1337

Open
Anikate-De opened this issue Jul 14, 2024 · 3 comments

Comments

@Anikate-De
Copy link
Contributor

Anikate-De commented Jul 14, 2024

This is a very specific issue faced while implementing the Interceptor interface in package: ok_http. The actual cause of this deadlock is yet to be found, (by a replica of Interceptor, as planned.)

Creating this issue to reference in PR and files: dart-lang/http#1257, to track the status.

cc: @HosseinYousefi

@HosseinYousefi
Copy link
Member

HosseinYousefi commented Jul 26, 2024

Basically if we're calling another callback on the same thread while hopping to another thread, it causes a deadlock.

For example, here we're recursively calling the same function but hopping from another thread:

    late final StringConverter stringConverter;
      stringConverter = StringConverter.implement($StringConverterImpl(
        parseToInt: (s) {
          final num = int.parse(s.toDartString(releaseOriginal: true));
          if (num > 0) {
            return StringConverterConsumer.consumeOnAnotherThreadAndLock(
                  stringConverter,
                  '${num - 1}'.toJString(),
                ).intValue() +
                1;
          }
          return 0;
        },
      ));
      StringConverterConsumer.consumeOnAnotherThreadAndLock(
        stringConverter,
        '5'.toJString(),
      );

Java code:

public interface StringConverter {
  int parseToInt(String s) throws StringConversionException;
}

public class StringConverterConsumer {
  public static Integer consumeOnSameThread(StringConverter stringConverter, String s) {
    try {
      return stringConverter.parseToInt(s);
    } catch (StringConversionException e) {
      return -1;
    }
  }

  public static Integer consumeOnAnotherThreadAndLock(StringConverter stringConverter, String s) {
    var futureTask = new FutureTask<>(() -> {
      return consumeOnSameThread(stringConverter, s);
    });
    new Thread(futureTask).start();
    try {
      return futureTask.get();
    } catch (InterruptedException | ExecutionException e) {
      throw new RuntimeException(e);
    }
  }
}

@dcharkes
Copy link
Collaborator

I don't really have enough context here. How those this problem arise? Why does the Java code have consumeOnAnotherThreadAndLock? If that's the Dart thread of an isolate getting locked that is not good. (You could possibly work around it by spawning a new helper isolate and using that. But in recursion getting all those threads locked is not going to work either because Dart has a limited thread pool.)

What use case causes this?

@HosseinYousefi
Copy link
Member

I don't really have enough context here. How those this problem arise?

OkHttp has a mechanism where you create request interceptors that get a Chain object. You can then call chain.proceed to invoke the next interceptor. This interceptor could be another implemented object on the same Dart thread, but chain.proceed returns a Response so it's blocking the thread which causes a deadlock on chain.proceed.

(You could possibly work around it by spawning a new helper isolate and using that. But in recursion getting all those threads locked is not going to work either because Dart has a limited thread pool.)

Yeah this is not ideal. For example we could have a redirect interceptor that handles http redirects, and there could be a lot of them.

I think we want to have a way to signal giving up control of the current function to be able to do something else and come back to this. Maybe we could use the existing async* + yield to do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants