Unsubscribe when thread is interrupted #1840
Conversation
roman-mazur
force-pushed
the
unsubscribe-on-interrupt
branch
from
71996e7 to
773fe03
Compare
|
I'll need to spend some time looking at this and it may not happen for 1.0, but I'll definitely spend time reviewing this. With a quick pass it looks okay, but due to the complexity of this use case I want to spend time playing with this code before merging. Thank you very much for contributing this! |
|
Let me know if I missed some blocking method. |
|
I'm going to put this on the 1.1 list as I'm not ready to make a change like this right before releasing 1.0 |
|
OK, thanks for looking at this. On 11:59, Sat, Nov 15, 2014 Ben Christensen [email protected]
|
| @@ -49,7 +50,7 @@ private BlockingOperatorLatest() { | |||
| @Override | |||
| public Iterator<T> iterator() { | |||
| LatestObserverIterator<T> lio = new LatestObserverIterator<T>(); | |||
| source.materialize().subscribe(lio); | |||
| lio.subscription = source.materialize().subscribe(lio); | |||
There was a problem hiding this comment.
This set races with the dereference of L101 and may cause NPE. Besides, since it would be set to the value lio essentially (plus a transparent SafeSubscriber layer), you don't really need this field. Just call this.unsubscribe() on L101.
roman-mazur
force-pushed
the
unsubscribe-on-interrupt
branch
from
773fe03 to
31cb642
Compare
roman-mazur
force-pushed
the
unsubscribe-on-interrupt
branch
from
31cb642 to
7020bcc
Compare
|
Edited per @akarnokd's remarks and rebased. |
|
Looks good to me now. Thanks. |
BlockingObservable: Unsubscribe when thread is interrupted
Originated from discussion #1804.
I was considering to make it opt-in, but it would drastically increase number of methods in public API.
What do you think?