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

EventCallerContainer not disposing IDisposable objects on scene switches #75

Closed
FodderMK opened this issue Feb 10, 2017 · 2 comments
Closed
Assignees
Labels
Milestone

Comments

@FodderMK
Copy link
Contributor

It looks like there's an issue with the order of operations when it comes to IDisposable objects on a scene switch. The EventCallerContainerExtension.OnUnregister() call currently looks like this:

public void OnUnregister(IInjectionContainer container) {
	container.afterAddBinding -= this.OnAfterAddBinding;
	container.bindingResolution -= this.OnBindingResolution;

	disposable.Clear();
	updateable.Clear();
	lateUpdateable.Clear();
	fixedUpdateable.Clear();
	focusable.Clear();
	pausable.Clear();
	quitable.Clear();

	if (behaviour != null && behaviour.gameObject != null) {
		MonoBehaviour.DestroyImmediate(behaviour.gameObject);
	}
	behaviour = null;
}

The problem is that when I switch scenes the OnUnregister() method appears to run before EventCallerBehaviour.OnDestroy() is run. This means the disposable.Clear() call occurs before the MonoBehaviour.DestroyImmediate() call, and THAT means the EventCallerBehaviour.OnDestroy() looks at an empty extension.disposable list and doesn't dispose of anything.

My temporary solution has been to change OnUnregister() to destroy the object before clearing the list:

public void OnUnregister(IInjectionContainer container) {
	container.afterAddBinding -= this.OnAfterAddBinding;
	container.bindingResolution -= this.OnBindingResolution;

	if (behaviour != null && behaviour.gameObject != null) {
		MonoBehaviour.DestroyImmediate(behaviour.gameObject);
	}

	disposable.Clear();
	[...]
}

I'm not sure what ramifications this has on the overall system though.

@intentor intentor self-assigned this Feb 14, 2017
@intentor intentor added the bug label Feb 14, 2017
@intentor intentor added this to the v2.24 milestone Feb 14, 2017
@intentor
Copy link
Owner

Thank you for your feedback, @FodderMK!

I'll look into the issue!

@intentor
Copy link
Owner

That's a neat solution, @FodderMK!

I'll apply it to the component!

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

No branches or pull requests

2 participants