You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by David Lecomber <ds...@ts.com> on 2002/01/13 14:12:36 UTC

[PATCH] serialization implmentation inconsistent with serializable/non-serializable attributes

Hi,

The writeObject method of StandardSession removes all the
non-null non-serializable attributes from the session.  This strikes me as
wrong, because a writing method shouldn't change the thing it writes..
Secondly if an attribute is null it's not persisted, nor is it added
to the attributes to unbind.  In StandardManger.unload() sessions are
expired after being serialized - which unbinds all the attributes..
Hence we don't need to do it in the first place..

Presently this is not a big issue because the writeObject is only
called from within a shutdown/reload anyway, but for future use I'd
like to suggest a patch.

David 

--- jakarta-tomcat-4.0.1-src/catalina/src/share/org/apache/catalina/session/StandardSession.java	Tue Nov 13 02:02:48 2001
+++ StandardSession.java	Sun Jan 13 13:05:23 2002
@@ -1,5 +1,5 @@
 /*
- * $Header: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/session/StandardSession.java,v 1.25 2001/07/31 02:00:02 craigmcc Exp $
+ * $Header: /home/cvspublic/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/session/StandardSession.java,v 1.25 2001/07/31 02:00:02 craigmcc Exp $
  * $Revision: 1.25 $
  * $Date: 2001/07/31 02:00:02 $
  *
@@ -1316,7 +1316,6 @@
         String keys[] = keys();
         ArrayList saveNames = new ArrayList();
         ArrayList saveValues = new ArrayList();
-        ArrayList unbinds = new ArrayList();
         for (int i = 0; i < keys.length; i++) {
             Object value = null;
             synchronized (attributes) {
@@ -1327,8 +1326,7 @@
             else if (value instanceof Serializable) {
                 saveNames.add(keys[i]);
                 saveValues.add(value);
-            } else
-                unbinds.add(keys[i]);
+            }
         }
 
         // Serialize the attribute count and the Serializable attributes
@@ -1348,16 +1346,9 @@
                 if (debug >= 2)
                     log("  storing attribute '" + saveNames.get(i) +
                         "' with value NOT_SERIALIZED");
-                unbinds.add(saveNames.get(i));
             }
         }
 
-        // Unbind the non-Serializable attributes
-        Iterator names = unbinds.iterator();
-        while (names.hasNext()) {
-            removeAttribute((String) names.next());
-        }
-
     }
 
 

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [PATCH] serialization implmentation inconsistent with serializable/non-serializable attributes

Posted by "Craig R. McClanahan" <cr...@apache.org>.

On Sun, 13 Jan 2002, David Lecomber wrote:

> Date: Sun, 13 Jan 2002 13:12:36 +0000
> From: David Lecomber <ds...@ts.com>
> Reply-To: Tomcat Developers List <to...@jakarta.apache.org>, dsl@ts.com
> To: tomcat-dev@jakarta.apache.org
> Subject: [PATCH] serialization implmentation inconsistent with
>     serializable/non-serializable attributes
>
> Hi,
>
> The writeObject method of StandardSession removes all the
> non-null non-serializable attributes from the session.  This strikes me as
> wrong, because a writing method shouldn't change the thing it writes..
> Secondly if an attribute is null it's not persisted, nor is it added
> to the attributes to unbind.  In StandardManger.unload() sessions are
> expired after being serialized - which unbinds all the attributes..
> Hence we don't need to do it in the first place..
>

One of your issues got addressed by a clarification late in the Servlet
2.3 development cycle - there is now no such thing as a null attribute --
calling 'session.setAttribute("foo", null)' is treated as equivalent to
calling 'session.removeAttribute("foo")'.

The other idea (avoiding the double unbind) sounds quite reasonable.

> Presently this is not a big issue because the writeObject is only
> called from within a shutdown/reload anyway, but for future use I'd
> like to suggest a patch.
>
> David

Craig


>
> --- jakarta-tomcat-4.0.1-src/catalina/src/share/org/apache/catalina/session/StandardSession.java	Tue Nov 13 02:02:48 2001
> +++ StandardSession.java	Sun Jan 13 13:05:23 2002
> @@ -1,5 +1,5 @@
>  /*
> - * $Header: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/session/StandardSession.java,v 1.25 2001/07/31 02:00:02 craigmcc Exp $
> + * $Header: /home/cvspublic/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/session/StandardSession.java,v 1.25 2001/07/31 02:00:02 craigmcc Exp $
>   * $Revision: 1.25 $
>   * $Date: 2001/07/31 02:00:02 $
>   *
> @@ -1316,7 +1316,6 @@
>          String keys[] = keys();
>          ArrayList saveNames = new ArrayList();
>          ArrayList saveValues = new ArrayList();
> -        ArrayList unbinds = new ArrayList();
>          for (int i = 0; i < keys.length; i++) {
>              Object value = null;
>              synchronized (attributes) {
> @@ -1327,8 +1326,7 @@
>              else if (value instanceof Serializable) {
>                  saveNames.add(keys[i]);
>                  saveValues.add(value);
> -            } else
> -                unbinds.add(keys[i]);
> +            }
>          }
>
>          // Serialize the attribute count and the Serializable attributes
> @@ -1348,16 +1346,9 @@
>                  if (debug >= 2)
>                      log("  storing attribute '" + saveNames.get(i) +
>                          "' with value NOT_SERIALIZED");
> -                unbinds.add(saveNames.get(i));
>              }
>          }
>
> -        // Unbind the non-Serializable attributes
> -        Iterator names = unbinds.iterator();
> -        while (names.hasNext()) {
> -            removeAttribute((String) names.next());
> -        }
> -
>      }
>
>
>
> --
> To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
>
>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>