PDA

View Full Version : why call initPlugin() from enabled()



mherger
2005-08-20, 13:30
Dean,

In change 3840 you added something like the following note:

> - Some version checks in plugins that may prevent issues when
> downgrading.

....and did the following check:

> +sub enabled {
> + return ($::VERSION ge '6.1') && initPlugin();
> +}
> +

I don't think we should call initPlugin() at this stage. The
initialisation is done later in Buttons::Plugins, after checking strings
etc.. It is therefore run twice at startup.

What is the idea behind calling initPlugin() in this place?

BTW: ShoutcastBrowser will always return true from initPlugin(). Calling
it to see whether we can enable it or not doesn't make much sense...

--

Michael

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

dean
2005-08-20, 19:17
It was my belief at the time that initPlugin's return value should
enable/disable the plugin, I think I cribbed that behavior from
another plugin.

If it's wrong, please fix (or let me know so I can.)

-dean




On Aug 20, 2005, at 1:30 PM, Michael Herger wrote:

> Dean,
>
> In change 3840 you added something like the following note:
>
>
>> - Some version checks in plugins that may prevent issues when
>> downgrading.
>>
>
> ...and did the following check:
>
>
>> +sub enabled {
>> + return ($::VERSION ge '6.1') && initPlugin();
>> +}
>> +
>>
>
> I don't think we should call initPlugin() at this stage. The
> initialisation is done later in Buttons::Plugins, after checking
> strings etc.. It is therefore run twice at startup.
>
> What is the idea behind calling initPlugin() in this place?
>
> BTW: ShoutcastBrowser will always return true from initPlugin().
> Calling it to see whether we can enable it or not doesn't make much
> sense...
>
> --
>
> Michael
>
> -----------------------------------------------------------
> Help translate SlimServer by using the
> StringEditor Plugin (http://www.herger.net/slim/)
>

Dan Sully
2005-08-20, 19:53
* dean blackketter shaped the electrons to say...

>It was my belief at the time that initPlugin's return value should
>enable/disable the plugin, I think I cribbed that behavior from
>another plugin.
>
>If it's wrong, please fix (or let me know so I can.)

It's wrong - the initPlugin() function is called at a later time.

-D
--
<iNoah> kernel's original recipe: 11 secret args and switches

mherger
2005-08-21, 01:19
>> It was my belief at the time that initPlugin's return value should
>> enable/disable the plugin, I think I cribbed that behavior from
>> another plugin.
>>
>> If it's wrong, please fix (or let me know so I can.)
>
> It's wrong - the initPlugin() function is called at a later time.

I'll take a look at them, thanks.

--

Michael

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

dean
2005-08-21, 08:07
Can initPlugin cause a plugin to fail? We should probably make it
possible to do so and not cause the server to crash.

On Aug 20, 2005, at 7:53 PM, Dan Sully wrote:

> * dean blackketter shaped the electrons to say...
>
>
>> It was my belief at the time that initPlugin's return value
>> should enable/disable the plugin, I think I cribbed that behavior
>> from another plugin.
>>
>> If it's wrong, please fix (or let me know so I can.)
>>
>
> It's wrong - the initPlugin() function is called at a later time.
>
> -D
> --
> <iNoah> kernel's original recipe: 11 secret args and switches
>

mherger
2005-08-21, 12:34
> Can initPlugin cause a plugin to fail?

Oh, yeah, it does.

> We should probably make it possible to do so and not cause the server to
> crash.

You mean eval() it or something?

--

Michael

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

dean
2005-08-21, 16:53
On Aug 21, 2005, at 12:34 PM, Michael Herger wrote:

>> Can initPlugin cause a plugin to fail?
>>
>
> Oh, yeah, it does.
>
>
>> We should probably make it possible to do so and not cause the
>> server to crash.
>>
>
> You mean eval() it or something?
Exactly.

Dan Sully
2005-08-21, 19:36
* dean blackketter shaped the electrons to say...

>>Oh, yeah, it does.
>>
>>>We should probably make it possible to do so and not cause the
>>>server to crash.
>>>
>>
>>You mean eval() it or something?
>Exactly.

It'd be better if they were re-entrant, and would just set a $iveBeenInitialized flag.

-D
--
<iNoah> you know, most free operating systems come preinstalled with their own high horse.

kdf
2005-08-21, 19:51
On 21-Aug-05, at 7:36 PM, Dan Sully wrote:

> * dean blackketter shaped the electrons to say...
>
>>> Oh, yeah, it does.
>>>
>>>> We should probably make it possible to do so and not cause the
>>>> server to crash.
>>>>
>>>
>>> You mean eval() it or something?
>> Exactly.
>
> It'd be better if they were re-entrant, and would just set a
> $iveBeenInitialized flag.
>
some set $initialized, and should skip out right away if called and
$initialzed is already set
-k

mherger
2005-08-21, 21:22
>>>> We should probably make it possible to do so and not cause the
>>>> server to crash.
>>> You mean eval() it or something?
>> Exactly.
>
> It'd be better if they were re-entrant, and would just set a
> $iveBeenInitialized flag.

As Kevin mentions most are re-entrant, and quite a few set that flag. But
this has nothing to do with Dean's idea about having initPlugin() wrapped
in eval(), does it?

--

Michael

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

Dan Sully
2005-08-22, 10:14
* Michael Herger shaped the electrons to say...

>As Kevin mentions most are re-entrant, and quite a few set that flag. But
>this has nothing to do with Dean's idea about having initPlugin() wrapped
>in eval(), does it?

It does, in that if you can re-enter initPlugin(), eval isn't needed.

-D
--
<weezyl> Oh, I cook bacon naked all the time. You just have to keep the heat on med-low.

mherger
2005-08-22, 13:25
>> As Kevin mentions most are re-entrant, and quite a few set that flag.
>> But this has nothing to do with Dean's idea about having initPlugin()
>> wrapped in eval(), does it?
>
> It does, in that if you can re-enter initPlugin(), eval isn't needed.

The plugin can crash (which renders it non re-entrant) without bringing
the server to a halt.

This patch allows for initPlugin() to crash without negatively affecting
the server.

--

Michael

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

Dan Sully
2005-08-22, 13:26
* Michael Herger shaped the electrons to say...

>>It does, in that if you can re-enter initPlugin(), eval isn't needed.
>
>The plugin can crash (which renders it non re-entrant) without bringing
>the server to a halt.
>
>This patch allows for initPlugin() to crash without negatively affecting the server.

Looks good.

-D
--
Minds are like parachutes... they work best when open.