You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "Attila Király (JIRA)" <ji...@apache.org> on 2011/01/07 11:11:00 UTC
[jira] Created: (WICKET-3311) Delete unused IPageSerializer & co
from o.a.w.Page class
Delete unused IPageSerializer & co from o.a.w.Page class
--------------------------------------------------------
Key: WICKET-3311
URL: https://issues.apache.org/jira/browse/WICKET-3311
Project: Wicket
Issue Type: Improvement
Affects Versions: 1.5-M3
Reporter: Attila Király
Priority: Minor
It seems the Page class holds remnants of some old (based on svn introcued in the first half of 2007) page serialization infrastructure that is no longer in use.
- Definition of IPageSerializer interface. It is not implemented or referenced anywhere outside Page class.
- public static final ThreadLocal<IPageSerializer> serializer: It is not used anywhere outside Page class and it is only read in Page class itself. It is a problematic code anyway because it can lead to nasty memory leaks if not cleaned properly.
- protected writeReplace() and package protected readPageObject(): not overridden or invoked from anywhere.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (WICKET-3311) Delete unused IPageSerializer & co
from o.a.w.Page class
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3311?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Igor Vaynberg resolved WICKET-3311.
-----------------------------------
Resolution: Fixed
Fix Version/s: 1.5-M4
Assignee: Igor Vaynberg
> Delete unused IPageSerializer & co from o.a.w.Page class
> --------------------------------------------------------
>
> Key: WICKET-3311
> URL: https://issues.apache.org/jira/browse/WICKET-3311
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.5-M3
> Reporter: Attila Király
> Assignee: Igor Vaynberg
> Priority: Minor
> Fix For: 1.5-M4
>
> Attachments: WICKET-3311.patch
>
>
> It seems the Page class holds remnants of some old (based on svn introcued in the first half of 2007) page serialization infrastructure that is no longer in use.
> - Definition of IPageSerializer interface. It is not implemented or referenced anywhere outside Page class.
> - public static final ThreadLocal<IPageSerializer> serializer: It is not used anywhere outside Page class and it is only read in Page class itself. It is a problematic code anyway because it can lead to nasty memory leaks if not cleaned properly.
> - protected writeReplace() and package protected readPageObject(): not overridden or invoked from anywhere.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3311) Delete unused IPageSerializer & co
from o.a.w.Page class
Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3311?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12979628#action_12979628 ]
Igor Vaynberg commented on WICKET-3311:
---------------------------------------
thanks.
> Delete unused IPageSerializer & co from o.a.w.Page class
> --------------------------------------------------------
>
> Key: WICKET-3311
> URL: https://issues.apache.org/jira/browse/WICKET-3311
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.5-M3
> Reporter: Attila Király
> Assignee: Igor Vaynberg
> Priority: Minor
> Fix For: 1.5-M4
>
> Attachments: WICKET-3311.patch
>
>
> It seems the Page class holds remnants of some old (based on svn introcued in the first half of 2007) page serialization infrastructure that is no longer in use.
> - Definition of IPageSerializer interface. It is not implemented or referenced anywhere outside Page class.
> - public static final ThreadLocal<IPageSerializer> serializer: It is not used anywhere outside Page class and it is only read in Page class itself. It is a problematic code anyway because it can lead to nasty memory leaks if not cleaned properly.
> - protected writeReplace() and package protected readPageObject(): not overridden or invoked from anywhere.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (WICKET-3311) Delete unused IPageSerializer & co
from o.a.w.Page class
Posted by "Attila Király (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3311?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Attila Király updated WICKET-3311:
----------------------------------
Attachment: WICKET-3311.patch
Attaching patch made against trunk. Tests pass.
> Delete unused IPageSerializer & co from o.a.w.Page class
> --------------------------------------------------------
>
> Key: WICKET-3311
> URL: https://issues.apache.org/jira/browse/WICKET-3311
> Project: Wicket
> Issue Type: Improvement
> Affects Versions: 1.5-M3
> Reporter: Attila Király
> Priority: Minor
> Attachments: WICKET-3311.patch
>
>
> It seems the Page class holds remnants of some old (based on svn introcued in the first half of 2007) page serialization infrastructure that is no longer in use.
> - Definition of IPageSerializer interface. It is not implemented or referenced anywhere outside Page class.
> - public static final ThreadLocal<IPageSerializer> serializer: It is not used anywhere outside Page class and it is only read in Page class itself. It is a problematic code anyway because it can lead to nasty memory leaks if not cleaned properly.
> - protected writeReplace() and package protected readPageObject(): not overridden or invoked from anywhere.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (WICKET-3311) Delete unused IPageSerializer & co
from o.a.w.Page class
Posted by "Attila Király (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3311?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Attila Király updated WICKET-3311:
----------------------------------
Component/s: wicket
Issue Type: Bug (was: Improvement)
After more analysis I think this is actually a bug not an improvement.
A comparison about the mentioned code pieces in the 1.4 and in the trunk (1.5):
- Page.serializer public static final ThreadLocal variable in 1.4 is used by protocol.http.pagestore.AbstractPageStore class and the Page class itself. However in 1.5 AbstractPageStore is deleted so the variable is only used and only for read by the Page class. This means that it is probably holding a null value all time.
- Page.IPageSerializer interface is implemented in 1.4 with protocol.http.pagestore.AbstractPageStore.PageSerializer class. However in 1.5 this PageSerializer class (and the enclosing AbstractPageStore class) is deleted.
- Page.IPageSerializer.deserializePage() method is used in 1.4 trough Page.readPageObject() which is invoked in Component.readObject(). However in 1.5 Component.readObject() is deleted so both Page.IPageSerializer.deserializePage() and Page.readPageObject() are not used anymore.
- Page.IPageSerializer.serializePage() method is used in 1.4 trough Page.writePageObject(). However in 1.5 Page.writePageObject() is deleted so Page.IPageSerializer.serializePage() is not used anymore.
- Page.IPageSerializer.getPageReplacementObject() method is used in 1.4 trough Page.writeReplace(). As Martin pointed it out this is part of Java serialization so it is stil called in wicket 1.5 too. But it does nothing when Page.serializer is not set which is the case for 1.5, as mentioned above.
If Page.serializer would be set by someone outside of wicket (seems very unlikely to me seeing how it was used in AbstractPageStore.serializePage() for example) it would break page serialization in 1.5 as it is only used by the serialization and not during deserialization.
So it seems these were part of the 1.4 page serialization mechanism and they were only partially removed in the rewritten 1.5 version. Imho they should be deleted (see patch).
> Delete unused IPageSerializer & co from o.a.w.Page class
> --------------------------------------------------------
>
> Key: WICKET-3311
> URL: https://issues.apache.org/jira/browse/WICKET-3311
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.5-M3
> Reporter: Attila Király
> Priority: Minor
> Attachments: WICKET-3311.patch
>
>
> It seems the Page class holds remnants of some old (based on svn introcued in the first half of 2007) page serialization infrastructure that is no longer in use.
> - Definition of IPageSerializer interface. It is not implemented or referenced anywhere outside Page class.
> - public static final ThreadLocal<IPageSerializer> serializer: It is not used anywhere outside Page class and it is only read in Page class itself. It is a problematic code anyway because it can lead to nasty memory leaks if not cleaned properly.
> - protected writeReplace() and package protected readPageObject(): not overridden or invoked from anywhere.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3311) Delete unused IPageSerializer & co
from o.a.w.Page class
Posted by "Martin Grigorov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3311?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12978723#action_12978723 ]
Martin Grigorov commented on WICKET-3311:
-----------------------------------------
Be careful with this. At least the last point in the description is wrong.
These methods are special for Java Serialization mechanisms and are used automatically by Object(Input|Output)Stream.
> Delete unused IPageSerializer & co from o.a.w.Page class
> --------------------------------------------------------
>
> Key: WICKET-3311
> URL: https://issues.apache.org/jira/browse/WICKET-3311
> Project: Wicket
> Issue Type: Improvement
> Affects Versions: 1.5-M3
> Reporter: Attila Király
> Priority: Minor
> Attachments: WICKET-3311.patch
>
>
> It seems the Page class holds remnants of some old (based on svn introcued in the first half of 2007) page serialization infrastructure that is no longer in use.
> - Definition of IPageSerializer interface. It is not implemented or referenced anywhere outside Page class.
> - public static final ThreadLocal<IPageSerializer> serializer: It is not used anywhere outside Page class and it is only read in Page class itself. It is a problematic code anyway because it can lead to nasty memory leaks if not cleaned properly.
> - protected writeReplace() and package protected readPageObject(): not overridden or invoked from anywhere.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (WICKET-3311) Delete unused
IPageSerializer & co from o.a.w.Page class
Posted by "Attila Király (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3311?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12978732#action_12978732 ]
Attila Király edited comment on WICKET-3311 at 1/7/11 5:46 AM:
---------------------------------------------------------------
You are right about writeReplace(). It's a standard method of java serialization and I forgot that (thanks for mentioning). However readPageObject() is not. It would be readResolve().
It seems that using custom page serializers is broken (because it is only taken into account by writes). If a custom serializer is not set the writeReplace() does nothing.
So I think the patch is still correct.
was (Author: akiraly):
You are right about writeReplace(). It's a standard method of ajva serialization and I forgot that (thanks for mentioning). However readPageObject() is not. It would be readResolve().
It seems that using custom page serializers is broken (because it is only taken into account by writes). If a custom serializer is not set the writeReplace() does nothing.
So I think the patch is still correct.
> Delete unused IPageSerializer & co from o.a.w.Page class
> --------------------------------------------------------
>
> Key: WICKET-3311
> URL: https://issues.apache.org/jira/browse/WICKET-3311
> Project: Wicket
> Issue Type: Improvement
> Affects Versions: 1.5-M3
> Reporter: Attila Király
> Priority: Minor
> Attachments: WICKET-3311.patch
>
>
> It seems the Page class holds remnants of some old (based on svn introcued in the first half of 2007) page serialization infrastructure that is no longer in use.
> - Definition of IPageSerializer interface. It is not implemented or referenced anywhere outside Page class.
> - public static final ThreadLocal<IPageSerializer> serializer: It is not used anywhere outside Page class and it is only read in Page class itself. It is a problematic code anyway because it can lead to nasty memory leaks if not cleaned properly.
> - protected writeReplace() and package protected readPageObject(): not overridden or invoked from anywhere.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-3311) Delete unused IPageSerializer & co
from o.a.w.Page class
Posted by "Attila Király (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-3311?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12978732#action_12978732 ]
Attila Király commented on WICKET-3311:
---------------------------------------
You are right about writeReplace(). It's a standard method of ajva serialization and I forgot that (thanks for mentioning). However readPageObject() is not. It would be readResolve().
It seems that using custom page serializers is broken (because it is only taken into account by writes). If a custom serializer is not set the writeReplace() does nothing.
So I think the patch is still correct.
> Delete unused IPageSerializer & co from o.a.w.Page class
> --------------------------------------------------------
>
> Key: WICKET-3311
> URL: https://issues.apache.org/jira/browse/WICKET-3311
> Project: Wicket
> Issue Type: Improvement
> Affects Versions: 1.5-M3
> Reporter: Attila Király
> Priority: Minor
> Attachments: WICKET-3311.patch
>
>
> It seems the Page class holds remnants of some old (based on svn introcued in the first half of 2007) page serialization infrastructure that is no longer in use.
> - Definition of IPageSerializer interface. It is not implemented or referenced anywhere outside Page class.
> - public static final ThreadLocal<IPageSerializer> serializer: It is not used anywhere outside Page class and it is only read in Page class itself. It is a problematic code anyway because it can lead to nasty memory leaks if not cleaned properly.
> - protected writeReplace() and package protected readPageObject(): not overridden or invoked from anywhere.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.