Discussion:
[FreeMarker-user] FileTemplateLoader doesnt close reader ?
Albert Kam
2013-03-12 07:20:18 UTC
Permalink
I took a peek at FileTemplateLoader.java and notices that
the closeTemplateSource() is empty method.
Is it okay not to close the Reader returned from getReader() ?

I am asking this because i am trying to mimic the best practice when
creating a custom TemplateLoader.

In my case, when following the instruction to auto-escape in here :
http://watchitlater.com/blog/2011/10/default-html-escape-using-freemarker/ :

- I found out that closing the reader returns an empty page after the
process. Commenting the IOUtils.closeQuietly() solves the empty page
problem.
- So i thought maybe i should close the reader (maybe through a ThreadLocal
storing the reader) in the closeTemplateSource()
- So i peeked the FileTemplateLoader for an example, and thus this post to
clarify.

Please share your thoughts.

Thank you,
Albert Kam
--
Do not pursue the past. Do not lose yourself in the future.
The past no longer is. The future has not yet come.
Looking deeply at life as it is in the very here and now,
the practitioner dwells in stability and freedom.
(Thich Nhat Hanh)
Daniel Dekany
2013-03-12 20:30:17 UTC
Permalink
I took a peek at FileTemplateLoader.java and notices that the
closeTemplateSource() is empty method.
Is it okay not to close the Reader returned from getReader() ?
The Reader is closed by whoever has called getReader() by calling
java.io.Reader.close(), not with a TemplateLoader method. The
closeTemplateSource() just closes the "template source", which in the
case of FileTemplateLoader is a File, and hence there's nothing to
close in it.
I am asking this because i am trying to mimic the best practice
when creating a custom TemplateLoader.
- I found out that closing the reader returns an empty page after
the process. Commenting the IOUtils.closeQuietly() solves the empty page problem.
That sounds impossible. By the time IOUtils.closeQuietly() is called,
the StringReader and the String it reads is already done. Maybe
IOUtils.closeQuietly() throws an exception that's improperly handled,
gibing an empty page?
- So i thought maybe i should close the reader (maybe through a
ThreadLocal storing the reader) in the closeTemplateSource()
You should not close the Reader that you return to the caller. But in
this case you have a Reader that you don't return to the caller or to
anybody, so of course you have to close it yourself, but that you
should do right where it's done on that blog.
- So i peeked the FileTemplateLoader for an example, and thus this post to clarify.
Please share your thoughts.
That implementation is not very good, because:

- It can't handle the case when the template already has an #ftl
header

- That it pre-loads the template, instead of creating a wrapping
Reader that appends and prepends what's needed right in the
character stream. Although it's not a big deal in practice, it's not
very nice.

You may look into the source code of FMPP, and there
Engine.wrapReader. (I was substantially younger when I wrote that, but
let's hope it's OK... :) )
Thank you,
Albert Kam
--
Best regards,
Daniel Dekany
Albert Kam
2013-03-13 02:42:36 UTC
Permalink
Post by Daniel Dekany
The Reader is closed by whoever has called getReader() by calling
java.io.Reader.close(), not with a TemplateLoader method. The
closeTemplateSource() just closes the "template source", which in the
case of FileTemplateLoader is a File, and hence there's nothing to
close in it.
That sounds impossible. By the time IOUtils.closeQuietly() is called,
the StringReader and the String it reads is already done. Maybe
IOUtils.closeQuietly() throws an exception that's improperly handled,
gibing an empty page?
I tried closing the reader and catching and logging the exception instead
of just using IOTools.closeQuietly(),
and notices that there's no exception happening, and still gave me the
empty page.
Commenting the close reader section gave me back the output.
Post by Daniel Dekany
You should not close the Reader that you return to the caller. But in
this case you have a Reader that you don't return to the caller or to
anybody, so of course you have to close it yourself, but that you
should do right where it's done on that blog.
Aha, maybe that's the reason it gave me the empty output.
According to
http://docs.oracle.com/javase/7/docs/api/java/io/StringReader.html#close()
"Closes the stream and releases any system resources associated with it.
Once the stream has been closed, further read(), ready(), mark(), or
reset() invocations will throw an IOException. Closing a previously closed
stream has no effect."
Maybe the caller of the closed-reader got an IOException somewhere and
hence, the empty output ?
Post by Daniel Dekany
- It can't handle the case when the template already has an #ftl
header
- That it pre-loads the template, instead of creating a wrapping
Reader that appends and prepends what's needed right in the
character stream. Although it's not a big deal in practice, it's not
very nice.
You may look into the source code of FMPP, and there
Engine.wrapReader. (I was substantially younger when I wrote that, but
let's hope it's OK... :) )
I actually have switched back to the normal FileTemplateLoader without
wrapping the escape directive for several reasons :

- I actually make use of functions for URLs generation.
And it's ugly to see the noescape tags wrapping every link calls, some
thing like :
<#noescape>${url.home("Return to main page")}</#noescape>
I prefer using ?html as needed

- Because i am using JBoss Tools for the freemarker editor (which does help
me in parsing error detection, highlighting, etc), it errors the line of
every noescape because it's not inside an escape directive, which gives me
the tension i dont need.

And thank you for the wrapReader sharing, i will add it to my library in
case i find a use of it in the future.

Regards from Jakarta,
Albert Kam
--
Do not pursue the past. Do not lose yourself in the future.
The past no longer is. The future has not yet come.
Looking deeply at life as it is in the very here and now,
the practitioner dwells in stability and freedom.
(Thich Nhat Hanh)
Daniel Dekany
2013-03-13 20:06:24 UTC
Permalink
Wednesday, March 13, 2013, 3:42:36 AM, Albert Kam wrote:

[snip]
Post by Daniel Dekany
I tried closing the reader and catching and logging the exception
instead of just using IOTools.closeQuietly(),
and notices that there's no exception happening, and still gave me the empty page.
Commenting the close reader section gave me back the output.
Are you returning a StringReader before calling
IOTools.closeQuietly()on *another* reader? Because then this
phenomenon it doesn't make any sense.

[snip]
Post by Daniel Dekany
Aha, maybe that's the reason it gave me the empty output.
According to
http://docs.oracle.com/javase/7/docs/api/java/io/StringReader.html#close()
"Closes the stream and releases any system resources associated with it.
Once the stream has been closed, further read(), ready(), mark(),
or reset() invocations will throw an IOException. Closing a
previously closed stream has no effect."
Maybe the caller of the closed-reader got an IOException somewhere and hence, the empty output ?
You aren't supposed to close the StringReader inside the
TemplateLoader. You should close the *other* reader, the one that you
don't return. Also, if FreeMarker gives empty output when an
IOException occurs, that's a bug. But I hope it doesn't do that, it's
rather the framework that calls it that suppresses that exception
instead of giving a HTTP 500.

[snip]
Post by Daniel Dekany
I actually have switched back to the normal FileTemplateLoader
- I actually make use of functions for URLs generation.
<#noescape>${url.home("Return to main page")}</#noescape>
I prefer using ?html as needed
It's a strange coincidence that you say this just after 2 weeks or so
after I have proposed #p. It addresses exactly this annoyance, but I
don't remember anybody but myself ever brought this up, till now.
Anyway, it will be in 2.3.20 most probably.
Post by Daniel Dekany
- Because i am using JBoss Tools for the freemarker editor (which
does help me in parsing error detection, highlighting, etc), it
errors the line of every noescape because it's not inside an escape
directive, which gives me the tension i dont need.
This will also be solved by #p, since then you won't need #noescpe
anymore. Indeed, I might as well deprecate #noescape then.
Post by Daniel Dekany
And thank you for the wrapReader sharing, i will add it to my
library in case i find a use of it in the future.
Regards from Jakarta,
Albert Kam
--
Best regards,
Daniel Dekany
Albert Kam
2013-03-14 02:36:45 UTC
Permalink
Post by Daniel Dekany
You aren't supposed to close the StringReader inside the
TemplateLoader.
Yeah, i returned a closed stringReader. What an omg-mistake !
Now i have changed it to close the 'wrapped' reader instead.
Thanks so much for the guides.
Post by Daniel Dekany
[snip]
Post by Daniel Dekany
I tried closing the reader and catching and logging the exception
instead of just using IOTools.closeQuietly(),
and notices that there's no exception happening, and still gave me the
empty page.
Post by Daniel Dekany
Commenting the close reader section gave me back the output.
Are you returning a StringReader before calling
IOTools.closeQuietly()on *another* reader? Because then this
phenomenon it doesn't make any sense.
[snip]
Post by Daniel Dekany
Aha, maybe that's the reason it gave me the empty output.
According to
http://docs.oracle.com/javase/7/docs/api/java/io/StringReader.html#close()
Post by Daniel Dekany
"Closes the stream and releases any system resources associated with it.
Once the stream has been closed, further read(), ready(), mark(),
or reset() invocations will throw an IOException. Closing a
previously closed stream has no effect."
Maybe the caller of the closed-reader got an IOException somewhere and
hence, the empty output ?
You aren't supposed to close the StringReader inside the
TemplateLoader. You should close the *other* reader, the one that you
don't return. Also, if FreeMarker gives empty output when an
IOException occurs, that's a bug. But I hope it doesn't do that, it's
rather the framework that calls it that suppresses that exception
instead of giving a HTTP 500.
[snip]
Post by Daniel Dekany
I actually have switched back to the normal FileTemplateLoader
- I actually make use of functions for URLs generation.
And it's ugly to see the noescape tags wrapping every link calls, some
<#noescape>${url.home("Return to main page")}</#noescape>
I prefer using ?html as needed
It's a strange coincidence that you say this just after 2 weeks or so
after I have proposed #p. It addresses exactly this annoyance, but I
don't remember anybody but myself ever brought this up, till now.
Anyway, it will be in 2.3.20 most probably.
Post by Daniel Dekany
- Because i am using JBoss Tools for the freemarker editor (which
does help me in parsing error detection, highlighting, etc), it
errors the line of every noescape because it's not inside an escape
directive, which gives me the tension i dont need.
This will also be solved by #p, since then you won't need #noescpe
anymore. Indeed, I might as well deprecate #noescape then.
Post by Daniel Dekany
And thank you for the wrapReader sharing, i will add it to my
library in case i find a use of it in the future.
Regards from Jakarta,
Albert Kam
--
Best regards,
Daniel Dekany
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
FreeMarker-user mailing list
https://lists.sourceforge.net/lists/listinfo/freemarker-user
--
Do not pursue the past. Do not lose yourself in the future.
The past no longer is. The future has not yet come.
Looking deeply at life as it is in the very here and now,
the practitioner dwells in stability and freedom.
(Thich Nhat Hanh)
Albert Kam
2013-03-14 02:52:53 UTC
Permalink
Sorry almost forgot.

I think the #p solution would be a good addition to the arsenal of
freemarker,
if paired with the automatic escaping feature.

Will be excited for 2.3.20 updates, but for now,
i'm content with manual escaping the expressions on user-generated-content.

(OOT) Wishful thinking :
Daniel Dekany
2013-03-16 09:58:18 UTC
Permalink
Post by Albert Kam
Sorry almost forgot.
I think the #p solution would be a good addition to the arsenal of freemarker,
if paired with the automatic escaping feature.
Will be excited for 2.3.20 updates, but for now,
i'm content with manual escaping the expressions on user-generated-content.
Loading...