Skip to content

Conversation

LachlanMcKee
Copy link
Collaborator

Description

Closes #296

Check list

  • I have updated CHANGELOG.md if required.
  • I have updated documentation if required.


override fun onCreate(lifecycle: Lifecycle) {
lifecycle.subscribe(onCreate = { flushOutputCache() })
lifecycle.subscribe(onCreate = { flushOutputCache() }, onDestroy = { intakeDisposable?.dispose() })
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly we should log an error, or raise an exception if input/output is invoked after onDestroy is called, rather than just swallowing silently?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't skip onCreate callback. androidx.lifecycle.LifecycleRegistry.moveToState forbids going from initialised state to destroyed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO do not see it as a problem that we should take care of, swallowing is OK for me.


private fun switchToExhaust() {
intake.subscribe { exhaust.accept(it) }
intakeDisposable = intake.subscribe { exhaust.accept(it) }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intake.subscribe(exhaust)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense

@CherryPerry
Copy link
Collaborator

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.

@LachlanMcKee
Copy link
Collaborator Author

@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.

@LachlanMcKee LachlanMcKee added the bug Something isn't working label Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NodeConnector intake is not disposed when a node is destroyed

2 participants