Fixed threadpool+ProcessExit problem in shutdown flow

[roji]

Will post a description in mono-devel for discussion...

[roji]

No comment on the list, I'm posting my message here as well:

It seems there's a small problem with the way the runtime shuts down at the moment: the thread pool (and apparently other resources) are shut down before the ProcessExit signal is fired. This is the cause for the NLog bug
described here, since NLog relies on the threadpool being available in ProcessExit (works on Microsoft but not mono). To my mind this is a bug, although oddly enough there's a runtime test that appears to imply that it may be normal?

This pull request has a test that reproduces the problem, and a proposed patch which is probably insufficient since my first time in the runtime. The general idea is this:

• mono_runtime_shutdown(), which runs the ProcessExit event code is executed from mini_cleanup(), which is a pretty late step (after we shutdown the pool and other resources in thread_manage()).
• The same thing happens in System_Environment_Exit(), which again shuts down the thread pool before calling mono_runtime_quit() (which eventually triggers the mono_runtime_shutdown()).
• My working patch moves mono_runtime_shutdown() and mono_domain_finalizers() out of mini_cleanup() and into thread_manage() and System_Environment_Exit(), right before we shut down the thread pool and other stuff.

Somebody who actually understands everything in there should take a look at this... For one thing, the according to the comment in mini.c, mono_domain_finalize() also seems like it should be called earlier, but going so (next to mono_runtime_shutdown) triggered a segmentation fault for me.

Also, my patch duplicates code the two function calls, although other calls are already duplicated. It seems that a small cleanup/refactor could help eliminate this duplication, both across thread_manage() and System_Environment_Exit() and across mini and the interpreter (which doesn't trigger the ProcessExit event?) I'd be glad to go deeper into this if people think it's good/necessary.

[tanuva]

I experience the same issue. My application stops quitting properly as soon as I create a Logger instance.

[damageboy]

Hi Guys... Is anything happening with this PR?
I have 20 something projects using NLog that keep getting stuck because of this...
It would be great to have this maybe fixed in time for mono 3.0 stable release?
Pretty please?

[Iristyle]

FWIW, I'm using the above mentioned fix in an NLog / ServiceStack set of services under Mono 3 (meebey packages) / Ubuntu 12.04 and we haven't run into any issues.

Note however, we're still in internal staging / demo mode, and not yet heavy production use. So take my experience with a grain of salt.

[sebastiang]

We are experiencing this problem quite pervasively, with test suites running in Windows/CLR but hanging on exit on Linux/mono.

[Iristyle]

@sebastiang are you running into this with NLog / using the fix mentioned?

In an ASP.NET (or a ServiceStack app hosted under ASPNET):

protected void Application_End(object sender, EventArgs e)
{
  // HACK: Works around a Mono exit issue
  // http://nlog-project.org/2011/10
  // https://github.com/NLog/NLog/issues/118
  NLog.LogManager.Configuration = null;
}

Our app is being continuously deployed, and each time we redeploy, we stop the existing service (it's an Upstart job that runs fastcgi), re-deploy and restart.

[sebastiang]

We've been able to do this in our standalone app that runs on Linux, but we don't know of the right point at which we can do this to keep NUnit tests fixtures from failing, aside from putting it in a shutdown method on each and every one of them.

(Update: Having tried a few options, there appears to be no time during the NUnit test fixture itself that setting this configuration to null helps anything. This hack seems to have to be done later in execution, but without recompiling NUnit, I'm not sure how to pull that off.)

[alanmcgovern]

@kumpera, can you take a look at this?

[sebastiang]

Any news on this front?

[kumpera]

The patch could have cleaned up things a bit instead of spreading things around. It didn't handle shutdown during debugger sessions, which I'll fix locally.

[sebastiang]

Rodrigo, I'm very excited to try this locally. Thanks for giving it a look.

[roji]

Hi rodrigo, thanks for finding the time for this.

I completely agree about your comment, it seemed to me that the whole runtime shutdown process could use a refactoring, but I didn't think I was the person to be doing that :)