On Tue, 15 Nov 2022 08:56:57 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> The CancelRequest test was observed failing again after >> [JDK-8294916](https://bugs.openjdk.org/browse/JDK-8294916) was integrated: >> there is a small race condition window in the code that unregisters the >> request BodySubscriber when a request is cancelled: if the request >> cancellation happens after the body subscriber is registered but before it >> is subscribed it may not be unregistered. >> >> The solution is to register it only after it has been successfully >> subscribed. > > src/java.net.http/share/classes/jdk/internal/net/http/Http1Exchange.java line > 213: > >> 211: @Override >> 212: public void onSubscribed() { >> 213: exchange.registerResponseSubscriber(this); > > The `registerResponseSubscriber` in addition to registering it with the > client will also set a local `responseSubscriber` member. Do you think we > should `null` it out in the callbacks where we call the > `unregisterResponseSubscriber`? Good question. But I believe not: it wasn't needed before this change. The only place where we look at the subscriber is cancelImpl. Depending on the propagation of error conditions we might introduce a race if we did that - especially since we are unregistering the subscriber before propagating errors (if any). ------------- PR: https://git.openjdk.org/jdk/pull/11110
