Nanowar - session-scope serialization fails if app-scope container is not serializable

I've just realised there may be a problem with my suggested fix - my modification to ServletContainerListener is WebWork specific - it uses a new class (ApplicationScopeViaActionContextObjectReference) which uses WebWork's ActionContext.

Hi James,

this is not really a Pico bug - but a Nano one.

Also, the factory implementation you provide is completely
static driven - which is the evil of IoC.
Moreover, it could probably all sit in nanowar component.
If you feel there is functionality in Pico that is required
then best file an issue for Pico only.

Could you have a go at refactoring it along these lines
and open a new issue on http://jira.codehaus.org/browse/NANO
with the new CVS patch?

Thanks,
Mauro

Done with JIRA.

Should we simply require all containers to be consistent with serialisation?

Normally a container hierarchy should be serializable. The funny part starts when there's a need for serializing only a child container, but not its parent. Normally this is a security issue, since you can use this mechanism to move parts of a container hierarchy onto a new root container ...

Or have a second listener that exists solely to support non-serializable ones, or ones where serialization is chosen not to happen (bad Inglish sorry)

The idea of a separate listener is good - as it keeps behaviour consistent
and the functionality can be implemented as additional functionality.
That said, I'm tempted to put this issue on the backburner (ie post 1.0 release),
as I fail to see a pressing usecase for the scenario of inconsistent serialisation.

If anybody sees the urgency for it, please shout.

Bumped to 1.1, Alternate Listener to be added after 1.0 release

Actually, I think this is a worse issue – the Application level container should NOT be getting serialized with the session at all! (IMO). Just posted about this on the listserv. In short the issues are:

Session serialization of app container will result in:
-serialization of entire container hierarchy in clustered environments.
-Inconsistent container hierarchy in stop/start environments where the session is persisted across app restarts.

The following patch makes Session integration optional. For those that need it (Tomcat users in particular), you can work around the session problems by eliminating them alltogether. Instead of ServletContainerListener, use NanoWarContextListener instead.

Also, NanoWarSessionListener + NanoWarContextListener ='s ServletContainerListener.

Should be backwards compatible.

-Mike (R)

I think that the permanent fix should be splitting the constant parent/child hierarchies so that session is not permanently a child of application.

Instead, it should be wired together for only the duration of a Request. So the request filter would:

1 - Grab Application Level Container.
2 - Grab detached session level container.
3 - Build request-level container.

Somehow hook into the whole mess so that any requests for parent.getComponent*() in the request container would propagate to the detached session container, and if it propages further, then query the app level container.

At first I thought that we could just have the request filter register the session container with the app container and the request with the session at the beginning of the request, and deregister them at the end of the request. However, I realized that each registration would cause a whole new transmission of the session container over the wire in a clustered environment (including once again attempting a transmission of the app level container).

Does pico have anything that we could do this with? Would a proxy work so the request filter could intercept any calls to the parent pico and propagate it up to the next container?

-Mike (R)

Applied patch as a fix for release 1.0. A comprehensive solution is due for release 1.1.

Ok, here's my first stab at designing a solution. We want the same hierarchial container behavior without the explicit references from child to parent to allow for efficient serialization:

Solution #1:

Step 1: Modify MutablePicoContainer by adding:

/**
Implementations should be synchronized.
@param parent: A parent container, may be null if no parent is desired.
*/
void setParent(PicoContainer parent);

Step 2: Create new MutablePicoContainer: TransientParentPicoContainer. The parent reference is transient so changes won't necessarily mean serialization across the network.

Step 3: Add class gems.AggregatePicoContainer.

public class AggregatePicoContainer() implements Startable {
private MutablePicoContainer[] hierarchy;

//
// hierarchy[0] is topmost parent, hierarchy[hierarchy.lengh - 1] is deepest child container.
public AggregatePicoContainer(MutablePicoContainer[] hierarchy)

//Implementation of pico methods
public Object getComponentInstance(Object key)

//Other methods here.

public void start()

public void stop()

public void wireHierarchyTogether() {
for (int i =1 i < hierarchy.length - 1; i++)

}

public void breakHierarchy() {
for (int i =1 i < hierarchy.length - 1; i++)

}
}

Step 4: Modify NanoWAR Request Filter To have the following steps:
-Build Request level NanoContainer Container.

• Create AggregatePicoContainer with Args [AppContainer, SessionContainer, RequestContainer]
  -AggregatePicoContainer.start();
  -Perform Rest of the request.
  -AggregatePicoContainer.stop();

Issues:
In a clustered envrionment, session braodcast modules that aren't smart enough to recognize that a transient field is updated would still broadcast the session for each request, but at least no attempt to broadcast the app-level or request-level container would occur.

So the end result is that for performant clustered environments, you still would want to skip the use of a session container.

In a non-clustered environment, this would work well.

Solution #2:

Provide parent references to the getComponent*() methods to the PicoContainer API like so:

Object getComponentInstance(Object key, PicoContainer parentCallback);

Each time the container needed an object from its parent, it would call if (parentCallback != null) parentCallback.getComponentInstance(key);

So each request for a parent container could be handled by the Aggregate picocontainer so it can work its way up a component hierarchy.

Disadvantages: Much more complicated, it breaks the current concept of a clean picocontainer hierarchy, and would require a change to the Pico API.

Advantages: There is no state change in the Session-level picocontainer so there would be no over-the-wire transfers in clustered environments. The calls to the aggregate picocontainer would stay the same from the client programmer, and yet the hierarchial behavior is still preserved.

So what do you think of these options? Does anyone see any other way to get this proper behavior?

-Mike (R)

#1: see "svn diff -r 1376:1398 container/src/java/org/picocontainer/MutablePicoContainer.java"
setParent was explicitly removed, since this can be used to compromise the security.
#2: Too much API change

What's left? Try to use a TransientHotSwappingPicoContainerProxy. WDYT?

Well, originally, I thought that maybe something like that would work, but I don't see any way of coping with it and session without somehow having a reference to a container's session which would cause us to be back at square one.

Perhaps I'm missing something and you could elaborate on how you'd use it ot keep strong references away from the session yet stil have the hiearchy?

Thanks for giving it some thought!

-Mike

Ok, here's another thought: (Majorly pseudocode)

Make AppContainer, SessionContainer, and RequestContainer completely independent.

During Request Filter phase, create an AggregatingPicoContainer that pulls all the independent containers together and creates a pseudo hierarchy. This pico container is destroyed at the end of the request phase.

public class AggregatingPicoContainer implements PicoContainer {

private PicoContainer delegate;

// Hierachy is such that pico[0] == parent
// pico[1] is child of parent, etc.
// so pico[0] is app container, pico[1] session container, pico[2] request container.
public AggregatingPicoContainer(PicoContainer[] hierchy)

public PicoContainer getDelegate()

/**

• The purpose of this function is to 'clone' each container in the pseudo hierarchy and
• create a real hierarchy.
  */
  protected PicoContainer buildPico(PicoContainer[] hierarchy) { //For Each Container In the hierarchy. //Create a Mutable PicoContainer. If i == 0, it has no parent, if i > 0, pico has parent of pico created for (i - 1). //Visit hierarchy[i], for each Component Adapter register it into created mutable Mutable PicoContainer // }

  }

The downsides to this method are:
-potential performance hit. I have no idea how much CPU/memory visiting the component adapters and registering them in the aggregating container would be. Since this hit would be taken with each request, it could be potentially an issue, althought the biggest CPU hog, getClass() would be taken care of because each component adapter already has a reference to the component key.

-No container at any time other than request phase. So if you have a thread spawned off that tries to access the container hierarchy after the request has finished, you'll have a problem. I don't consider this a HUGE issue since:

(1) According to spec, you aren't supposed to spawn threads anyway. (Although, frankly, I ignore this quite often! )

(2) If you perform proper dependency injection, the spawned component should have no need to reference pico anyway.

What do you think??

-Mike (R)