On Nov 14, 2005, at 10:19 PM, Allen Gilliland wrote:
If the current instantiation problem is causing an outstanding bug
then I am fine with just commiting a smaller fix for 2.0, but I would
like to see something more elaborate happen in the long run.
+1 to that.
The PageHelper was changed to do the init at startup in CVS rev 1.22.
Here's the 1.21 to 1.22 diff:
<http://tinyurl.com/8obb3>
<https://roller.dev.java.net/source/browse/roller/roller/src/org/
roller/presentation/velocity/PageHelper.java?
r1=text&tr1=1.21&r2=text&tr2=1.22&diff_format=h>
I think we should back this change out before 2.0.
- Dave
-- Allen
Anil Gangolli wrote:
There's a bug (ROL-893) that highlights a fundamental problem in the
coding of the TopicTagPlugin, but also perhaps an issue in the plugin
interface and its instantiation model, which is what I wanted to
discuss.
Going back in the SVN history, it appears that the instantiation
model hasn't changed since 1.2, but I think it must have changed not
long before that.
The presence of init(RollerRequest rreq, VelocityContext ctx) in the
PagePlugin interface suggests that plugin instances should not be
shared across RollerRequests. I'm pretty sure this was the case in
some prior version. That is, I believe that the PageHelper was
creating new instances of each plugin class prior to calling init,
and this is also suggested by some of the variable names in
PageHelper (which refer to classes rather than instances).
However, now it appears (unless I am totally confused in my reading
of the code) that instances are created only once at servlet context
initialization, while the init() is still called per request. So if
a plugin happens to use init() to set up some member values, it can
cause unpredictable behavior now because the instance is shared. I
believe this is causing ROL-893, and it means a bit of rewrite and
possibly a decrease in the functionality of the TopicTagPlugin (with
respect to using user-specific bookmarks to allow defining alternate
tag sites).
I'm pretty sure I understood the instantiation model when I
originally wrote the plugin, and it was safe when I originally wrote
it, but now it is very unsafe. Did this behavior change around 1.1
- 1.2? Or am I confused? Does anyone else get what I'm talking
about with the expectation that having the current request and
velocity context in the init() call suggests to the implementor that
it wou't be shared?
--a.
