Skip to content

More Events for Reloading #86

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

Closed
schmitch opened this issue Oct 23, 2019 · 11 comments
Closed

More Events for Reloading #86

schmitch opened this issue Oct 23, 2019 · 11 comments
Labels
closed-stale Closed because the issue got stale and there wasn't any follow-up. enhancement New feature or request help wanted Extra attention is needed stale

Comments

@schmitch
Copy link

schmitch commented Oct 23, 2019

Hello, I'm trying to build a Plugin System on top of your awesome Library.
I currently have the following use cases:

  • Download & Start Plugins at runtime
  • Upgrade Plugins at runtime
  • Stop Plugins at runtime

while downloading, starting and stopping is actually working really nicely, reloading/upgrading has some caveheats.

currently I have uploaded a demo project at github here: https://github.com/schmitch/dotnet-plugin-demo

currently I need to have a Stop method called before reloading my plugin.
Unfortunatly I can only call it after the plugin got reloaded.

That is my log output:

Current Time V4: 23.10.2019 12:04:50
Reload Triggered, EventArgs: McMaster.NETCore.Plugins.PluginLoader / McMaster.NETCore.Plugins.PluginLoader
info: Microsoft.Hosting.Lifetime[0]
      Application is shutting down...
Created plugin instance 'Timestamp Plugin V5'.
Current Time V5: 23.10.2019 12:04:52

How it should be:

Current Time V4: 23.10.2019 12:04:50
info: Microsoft.Hosting.Lifetime[0]
      Application is shutting down...
Reload Triggered, EventArgs: McMaster.NETCore.Plugins.PluginLoader / McMaster.NETCore.Plugins.PluginLoader
Created plugin instance 'Timestamp Plugin V5'.
Current Time V5: 23.10.2019 12:04:52

Currently I think it would be possible by introducing an event that will be called, before this._context.Unload(); is called. Maybe call it BeforeReload (i.e. Reloaded could also be deprecated in favor of AfterReload, then)?

@schmitch
Copy link
Author

schmitch commented Oct 23, 2019

I also have the problem that I can't kill the application via. CTRL+C if I use a host-in-host (same repo, just the host-in-host) branch. It block forever (not sure why, since everything is unloaded)
i.e. it would be great to actually listen to the AssemblyLoadContext Event Unloading.

@natemcmaster natemcmaster added the more info needed Please provide more information. label Oct 26, 2019
@natemcmaster
Copy link
Owner

Why do you need need to call .Stop() before the assembly is reloaded? Can you elaborate more on what problem you're encountering?

I also have the problem that I can't kill the application via. CTRL+C if I use a host-in-host (same repo, just the host-in-host) branch.

Can you confirm that this problem is the result of DotNetCorePlugins? It seems to me like this problem would occur in a 'vanilla' project that isn't doing dynamic loading.

@schmitch
Copy link
Author

schmitch commented Oct 28, 2019

Why do you need need to call .Stop() before the assembly is reloaded? Can you elaborate more on what problem you're encountering?

Actually I need to call stop to correctly close everything in the plugin, like database connections.
Btw I created an example here: https://github.com/schmitch/dotnet-plugin-demo where I "create" a dotnet core host as a plugin, (i.e. the plugin could even start a web server that could be "hot" reloaded, but that means I need to stop it, before readding the new one)

Can you confirm that this problem is the result of DotNetCorePlugins? It seems to me like this problem would occur in a 'vanilla' project that isn't doing dynamic loading.

nope this happens with AssemblyLoadContext aswell, it's just harder to debug with DotNetCorePlugins, because it's impossible to get any info about Unloading. (i.e. when using AssemblyLoadContext directly I can attach to the Unloading event and debug the issues. (It would be good if DotNetCorePlugins can give me the option to look into it aswell.)

Both things are just features, i.e. more events that either help by debugging and it helps to have a clean shutdown before the plugin gets unloaded.

Hope this feedback helps?

@no-response no-response bot removed the more info needed Please provide more information. label Oct 28, 2019
@natemcmaster
Copy link
Owner

Actually I need to call stop to correctly close everything in the plugin, like database connections.

Makes sense, but what problems are you having with the current API? The reason I'm asking is that I don't yet understand a compelling reason to add a new event API. An important thing to note about AssemblyLoadContext.Unload() is that it doesn't force unload and guaranteed that it will be unloaded; it just initiates the unloading. The unloading actually happens later, after garbage collection and threads and references are cleaned up. (See docs for more details.) Based on that, I suspect your use case still works, but if it's not, I'd like to know what kinds of issues you see (besides the log output not appearing in the order you expect).

@dazinator
Copy link
Contributor

In terms of cleanup how about this:

  1. As Reload is a public method, you could set up your own file watcher / event to call it, but before you do call Reload, do your "cleanup". This could take the form:

A) Make sure you call Dispose() on any disposable instances loaded from that assy load context. This is easier if for example you have a DI container responsible for the lifetime of all those types, and then you can just Dispose() that container.

@sgf
Copy link

sgf commented Nov 9, 2021

In terms of cleanup how about this:

  1. As Reload is a public method, you could set up your own file watcher / event to call it, but before you do call Reload, do your "cleanup". This could take the form:

A) Make sure you call Dispose() on any disposable instances loaded from that assy load context. This is easier if for example you have a DI container responsible for the lifetime of all those types, and then you can just Dispose() that container.

cant do that without download full source code.
because some thing is internal like ManagedLoadContext.

anyway,i voted this. need an event to do release some resource for the plugin.

@sgf
Copy link

sgf commented Nov 11, 2021

  1. As Reload is a public method, you could set up your own file watcher / event to call it, but before you do call Reload, do your "cleanup". This could take the form:

u are right,i do own file watcher and call the Reload now. thanks for share your idea.

@dazinator
Copy link
Contributor

@sgf you are welcome, I do have some good ideas occasionally, not as often as I'd like ;-)

@natemcmaster natemcmaster added enhancement New feature or request help wanted Extra attention is needed labels Nov 13, 2021
@natemcmaster
Copy link
Owner

FYI - this project is in maintenance mode right now. I updated the labels on this issue. See #117 for detail on what the applied labels indicate.

@github-actions
Copy link

This issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please comment if you believe this should remain open, otherwise it will be closed in 14 days. Thank you for your contributions to this project.

@github-actions github-actions bot added the stale label Nov 14, 2022
@github-actions
Copy link

Closing due to inactivity.
If you are looking at this issue in the future and think it should be reopened, please make a commented here and mention natemcmaster so he sees the notification.

@github-actions github-actions bot added the closed-stale Closed because the issue got stale and there wasn't any follow-up. label Nov 29, 2022
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-stale Closed because the issue got stale and there wasn't any follow-up. enhancement New feature or request help wanted Extra attention is needed stale
Projects
None yet
Development

No branches or pull requests

4 participants