Issue #296 Dispose NodeConnector intake in onDestroy #297
Conversation
| @@ -31,7 +33,7 @@ class NodeConnector<Input, Output>( | |||
| } | |||
|
|
|||
| override fun onCreate(lifecycle: Lifecycle) { | |||
| lifecycle.subscribe(onCreate = { flushOutputCache() }) | |||
| lifecycle.subscribe(onCreate = { flushOutputCache() }, onDestroy = { intakeDisposable?.dispose() }) | |||
There was a problem hiding this comment.
Possibly we should log an error, or raise an exception if input/output is invoked after onDestroy is called, rather than just swallowing silently?
There was a problem hiding this comment.
You can't skip onCreate callback. androidx.lifecycle.LifecycleRegistry.moveToState forbids going from initialised state to destroyed.
There was a problem hiding this comment.
sorry, I wasn't talking about lifecycles here @CherryPerry. I meant if someone called input.accept/output.accept after onDestroy for the node was called.
In this case the output event would be ignored, but the input event would not.
There was a problem hiding this comment.
IMHO do not see it as a problem that we should take care of, swallowing is OK for me.
| @@ -55,7 +57,7 @@ class NodeConnector<Input, Output>( | |||
| } | |||
|
|
|||
| private fun switchToExhaust() { | |||
| intake.subscribe { exhaust.accept(it) } | |||
| intakeDisposable = intake.subscribe { exhaust.accept(it) } | |||
There was a problem hiding this comment.
intake.subscribe(exhaust)
|
Honestly do not think it is an error. Both source and subscriber are managed by us, they should be GCed together as soon as everybody unsubscribed from node. But not against PR, makes sense. |
|
@CherryPerry the garbage collector is not deterministic, so if we don't unsubscribe in a deterministic manner, users may get unexpected behaviours (depending on whether they are still sending events after a node is destroyed. |
Description
Closes #296
Check list
CHANGELOG.mdif required.