PDA

View Full Version : shutdownPlugin() vs. disablePlugin()



mherger
2005-08-22, 22:09
slimserver.pl is calling Slim::Buttons::Plugins::shutdownPlugins() on
cleanup. That routine calls a routine shutdownPlugins() in the plugins -
which is documented. But I haven't found one plugins that uses it. Instead
some use disablePlugin(), which is called from various places in
Buttons::Plugins and Web::Setup.

Are shutdownPlugin() and disablePlugin() meant to do the same? Shouldn't
we call them all the same and always use the same? I'd prefern
disablePlugin() in contrast to initPlugin()

--

Michael

-----------------------------------------------------------
Help translate SlimServer by using the
StringEditor Plugin (http://www.herger.net/slim/)

mherger
2005-08-23, 03:58
> slimserver.pl is calling Slim::Buttons::Plugins::shutdownPlugins() on
> cleanup. That routine calls a routine shutdownPlugins() in the
> plugins -
> which is documented. But I haven't found one plugins that uses it.
> Instead
> some use disablePlugin(), which is called from various places in
> Buttons::Plugins and Web::Setup.

Maybe a follow up question: should initPlugin() be re-run after
disablePlugin/shutdownPlugin() has been run and the plugin is
re-enabled?

--
Michael

-----------------------------------------------------------
Help translate SlimServer by using the
SlimString Translation Helper (http://www.herger.net/slim/)

mherger
2005-08-23, 20:51
> Are shutdownPlugin() and disablePlugin() meant to do the same? Shouldn't
> we call them all the same and always use the same? I'd prefern
> disablePlugin() in contrast to initPlugin()

Any idea anybody?!?

Out of about 40 plugins I found _one_ which is using shutdownPlugin()
(WebLogger) and about 3 or 4 which use disablePlugin() (mainly kdf's
works: iTunes, MusicMagic, MoodLogic, RandomPlay).

IMHO none of them is handled correctly as the API isn't clear.

--

Michael

-----------------------------------------------------------
Help translate SlimServer by using the
StringEditor Plugin (http://www.herger.net/slim/)

kdf
2005-08-23, 21:08
On 23-Aug-05, at 8:51 PM, Michael Herger wrote:

>> Are shutdownPlugin() and disablePlugin() meant to do the same?
>> Shouldn't we call them all the same and always use the same? I'd
>> prefern disablePlugin() in contrast to initPlugin()
>
> Any idea anybody?!?
>
> Out of about 40 plugins I found _one_ which is using shutdownPlugin()
> (WebLogger) and about 3 or 4 which use disablePlugin() (mainly kdf's
> works: iTunes, MusicMagic, MoodLogic, RandomPlay).
>
I believe shutdownPlugin was intended to be called as a cleanup if the
service is closing down. i am not sure what would require that. I
never really understood why it was there. I missed when it showed up.

disablePlugin was originally used to strip out menus, links, setup
groups, etc that are added during load that would need to be cleared
out if the plugin is later disabled. Most likely, any improper use is
due to changes in the init structure that no longer require those
functions. The API was created at the time that the importers were made
into plugins. They fit a void at the time and the description of the
API was left a little vague deliberately just to see what people might
want to do with it. All it really did was run when the plugin was
disabled. you could use it to send "sure, disable me NOW, you coward"
on the display if you wanted :)

-kdf

mherger
2005-08-23, 21:22
> I believe shutdownPlugin was intended to be called as a cleanup if the
> service is closing down.

Not only then. From the docs:

"shutdownPlugin()
subroutine to handle any operations that need to be performed to shut down
the plugin cleanly, or to be run when disabled."

Hmm... quite confusing. WebLogger is following this instruction - and
would therefore never kill its timers. We should be clearer here.

> you could use it to send "sure, disable me NOW, you coward" on the
> display if you wanted :)

I'll have to update my plugins!

--

Michael

-----------------------------------------------------------
Help translate SlimServer by using the
StringEditor Plugin (http://www.herger.net/slim/)

kdf
2005-08-23, 22:28
On 23-Aug-05, at 9:22 PM, Michael Herger wrote:

>> I believe shutdownPlugin was intended to be called as a cleanup if
>> the service is closing down.
>
> Not only then. From the docs:
>
> "shutdownPlugin()
> subroutine to handle any operations that need to be performed to shut
> down the plugin cleanly, or to be run when disabled."
>
> Hmm... quite confusing. WebLogger is following this instruction - and
> would therefore never kill its timers. We should be clearer here.
>
>
right, i dont think i recall the server ever calling this function :)

Killing the server from command line, there is often a lot of 'stuff'
coughed up before you get a prompt again.

-k

mherger
2005-08-24, 00:25
> > Hmm... quite confusing. WebLogger is following this instruction -
and
> > would therefore never kill its timers. We should be clearer here.
> >
> right, i dont think i recall the server ever calling this function :)

Oh, it does! At shutdown :-)

--

Michael

-----------------------------------------------------------
Help translate SlimServer by using the
SlimString Translation Helper (http://www.herger.net/slim/)

mherger
2005-08-25, 10:57
Dan? Dean?

Bug or feature?

> slimserver.pl is calling Slim::Buttons::Plugins::shutdownPlugins() on
> cleanup. That routine calls a routine shutdownPlugins() in the plugins -
> which is documented. But I haven't found one plugins that uses it.
> Instead some use disablePlugin(), which is called from various places in
> Buttons::Plugins and Web::Setup.
>
> Are shutdownPlugin() and disablePlugin() meant to do the same? Shouldn't
> we call them all the same and always use the same? I'd prefern
> disablePlugin() in contrast to initPlugin()
>



--

Michael

-----------------------------------------------------------
Help translate SlimServer by using the
StringEditor Plugin (http://www.herger.net/slim/)

dean
2005-08-25, 14:33
I'd guess that initPlugin and shutdownPlugin are what gets called
when the server starts up and shuts down respectively.

I'd also expect that if we were enabling and disabling plugins while
the server was running, they'd also get called.

I can't think of a reason why disablePlugin would be called, so maybe
we should get rid of it.


On Aug 25, 2005, at 10:57 AM, Michael Herger wrote:

> Dan? Dean?
>
> Bug or feature?
>
>
>> slimserver.pl is calling Slim::Buttons::Plugins::shutdownPlugins()
>> on cleanup. That routine calls a routine shutdownPlugins() in the
>> plugins - which is documented. But I haven't found one plugins
>> that uses it. Instead some use disablePlugin(), which is called
>> from various places in Buttons::Plugins and Web::Setup.
>>
>> Are shutdownPlugin() and disablePlugin() meant to do the same?
>> Shouldn't we call them all the same and always use the same? I'd
>> prefern disablePlugin() in contrast to initPlugin()
>>
>>
>
>
>
> --
>
> Michael
>
> -----------------------------------------------------------
> Help translate SlimServer by using the
> StringEditor Plugin (http://www.herger.net/slim/)
>

mherger
2005-08-25, 21:09
> I can't think of a reason why disablePlugin would be called, so maybe we
> should get rid of it.

I'll then rename those disablePlugins() I know off and add a "depreciated"
message to where they are called (it's one single place now). Ok?

--

Michael

-----------------------------------------------------------
Help translate SlimServer by using the
StringEditor Plugin (http://www.herger.net/slim/)

Dan Sully
2005-08-25, 21:09
* Michael Herger shaped the electrons to say...

>>I can't think of a reason why disablePlugin would be called, so maybe we
>>should get rid of it.
>
>I'll then rename those disablePlugins() I know off and add a "depreciated"
>message to where they are called (it's one single place now). Ok?

Sounds good!

-D
--
<noah> I used to be indecisive, but now I'm not sure.

kdf
2005-09-07, 14:23
I think I've just noticed a problem with this.

iTunes, musicmagic amd moodlogic all used to use disablePlugin, which was based on the idea that this was run when the plugin was disabled. thus is set the use itunes (and the like) settings to 0. Now, with this being called on shutdown of the server, itunes, musicmagic and moodlogic are all unused on restart.

One option would be to leave them set as used, but then all tests must also make sure that the plugin is enabled AND set to use.

any thoughts?

mherger
2005-09-08, 12:47
Kevin,

I know I am late... Is change 4201 supposed to fix this problem?

> iTunes, musicmagic amd moodlogic all used to use disablePlugin, which
> was based on the idea that this was run when the plugin was disabled.
> thus is set the use itunes (and the like) settings to 0. Now, with
> this being called on shutdown of the server, itunes, musicmagic and
> moodlogic are all unused on restart.

--

Michael

-----------------------------------------------------------
Help translate SlimServer by using the
StringEditor Plugin (http://www.herger.net/slim/)

kdf
2005-09-08, 13:19
Quoting Michael Herger <slim (AT) herger (DOT) net>:

> Kevin,
>
> I know I am late... Is change 4201 supposed to fix this problem?

Not late at all :)

it is a stopgap, to at least allow the settings to hold over from a restart.
I'm still interested in hearing about any problems that might come up as a
result, say if a user disables one of the importers. Ideally, when disabled,
the server wont waste any cycles doing anyting mixer or importer related. I
haven't had enough time to dig for any signs. I went ahead with 4201 just to
get rid of the need to keep re-enabling and suffering the rescan.

-kdf

mherger
2005-09-08, 20:58
> it is a stopgap, to at least allow the settings to hold over from a
> restart.

Please help me a little bit... Is this that two step thing that's always
confusing me with these importers: first enable the plugin, then select it
to actually do what it's supposed to do? Or the other way round in this
case (de-select, disable)?

> I'm still interested in hearing about any problems that might come up as
> a result, say if a user disables one of the importers. Ideally, when
> disabled,
> the server wont waste any cycles doing anyting mixer or importer
> related. I
> haven't had enough time to dig for any signs. I went ahead with 4201
> just to
> get rid of the need to keep re-enabling and suffering the rescan.

One idea: check Slim::Utils::Prefs::getArray('disabledplugins') before
turning the plugin off in shutdownPlugin(). That flag should already been
checked when it is called.

--

Michael

-----------------------------------------------------------
Help translate SlimServer by using the
StringEditor Plugin (http://www.herger.net/slim/)

kdf
2005-09-08, 23:59
On 8-Sep-05, at 8:58 PM, Michael Herger wrote:

>> it is a stopgap, to at least allow the settings to hold over from a
>> restart.
>
> Please help me a little bit...
it is the Use itunes, or use Moodlogic setting that everyone is used to
looking for to make sure their itunes import is available and turned
on. The pref is used throughout the imports as a test.
-k