Re: Terminating provider process

Jan Safranek <[email protected]> Wed, 29 May 2013 15:47:16 +0200
Newsgroups gmane.network.open-pegasus.general
Message-ID <[email protected]>
I filled bug #9669 so it does not get lost.

Jan

On 05/28/2013 11:18 AM, Jan Safranek wrote:
> I finally dived into sources, luckily ProviderAgent looks simple enough.
> Its main loop is in ProviderAgent::run(), which can be simplified into:
> 
> ProviderAgent::run()
> {
> 	while (!_terminating) {
> 		active = _readAndProcessRequest();
> 		if (!active &&
> 			!_providerManagerRouter.hasActiveProviders() &&
> 			(_threadPool.runningCount() == 0))
> 		_terminating = true
> 	}
> }
> 
> 
> ProviderAgent::readAndProcessRequest()
> {
> 	// process other messages
> 	if (wakeup_message) {
> 		_unloadIdleProviders();
> 		return false
> 	}
> 	return true;
> }
> 
> and finally
> 
> ProviderAgent::_unloadIdleProviders()
> {
> 	// start a thread to unload idle providers
> }
> 
> So, the first wake up message just starts a thread to unload idle
> providers, which depends on provider speed and may take some time.
> Therefore the main run() loop continues processing next requests.
> Only after the second wake up message run() recognizes, that all
> providers are already unloaded and exits.
> 
> Attached highly experimental patch reworks _unloadIdleProviders() into:
> 
> ProviderAgent::_unloadIdleProviders()
> {
> 	// start a thread to unload idle providers
> 	// wait for the thread to finish
> }
> 
> Thus when readAndProcessRequest() returns false, all providers are
> already unloaded, run() checks it and exits the main loop. All this
> happens after the *first* wake up message.
> 
> 
> I hope I did not miss any error case or so.
> 
> In addition, the patch waits up to
> PEGASUS_DEFAULT_SHUTDOWN_TIMEOUT_SECONDS_STRING (=30) seconds for the
> providers to unload and the ProviderAgent cane get blocked for this time
> if a provider takes too much time to unload. The providers were idle for
> 5 minutes, so let's hope they can wait for another 30 seconds (in the
> worst case). It would be nice if this timeout was configurable, I am not
> sure how to do it.