Here's what I'd probably do:

Replace these calls with dedicated classes for each proxy type

        if (systemInstance.getComponent(HttpServletRequest.class) == null) {
            systemInstance.setComponent(HttpServletRequest.class, 
Proxys.threadLocalProxy(HttpServletRequest.class, 
OpenEJBSecurityListener.requests, null));
        }

To

        if (systemInstance.getComponent(HttpServletRequest.class) == null) {
            systemInstance.setComponent(HttpServletRequest.class, 
HttpServletRequestProxy.get());
        }

The `HttpServletRequestProxy` would be an interface that extends 
HttpServletRequest and Serializable, like this one:

 - 
https://github.com/apache/tomee/blob/main/server/openejb-client/src/main/java/org/apache/openejb/client/EJBHomeProxy.java

Unlike that interface, I'd put all the implementation code in the interface and 
add a static get() method.  The stub would look something like this:

    public interface HttpServletRequestProxy extends Serializable, 
HttpServletRequest {
    
        public Object writeReplace() throws ObjectStreamException;
    
        public static HttpServletRequest get() {
            // return a proxy that implements this interface and uses our 
Handler inner-class
        }
    
        public static class Handler implements InvocationHandler {
            // move the relevant Handler code in here and forget Proxys
            // add code to handle the writeReplace() method and return an 
instance of Serialized
        }
    
        public static class Serialized implements Serializable {
            public Object writeReplace() throws ObjectStreamException {
                    return HttpServletRequestProxy.get();
            }
        }
    }

What will happen is that when the HttpServletRequest is serialized, an instance 
of Serialized will be written instead.  On deserialization, the `Serialized` 
instance will replace itself again using the same 
`HttpServletRequestProxy.get()` call used to create the original proxy.

I'd do that for all the proxy types (HttpSession).  Each proxy type would have 
hardcoded references in their handler to the ThreadLocal static fields they 
need.

Then I would try to delete every line of code from Proxys that we could.

Incidentally, this looks like a bug:

 - 
https://github.com/apache/tomee/blob/tomee-8.x/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/TomcatWebAppBuilder.java#L362


-David

> On Aug 10, 2023, at 1:35 PM, Jonathan S. Fisher <exabr...@gmail.com> wrote:
> 
> Hello TomEE friends, I was looking to patch a bug we encountered and could
> use some advice from the other committers.
> 
> Given this CDI Bean:
> 
> @SessionScoped
> @Named
> public class MyPageBean implements Serializable {
> @Inject private HttpServletRequest req;
> ... use the bean somewhere in EL
> }
> 
> This will result in an exception when serializing the Session (For session
> replication or passivation): java.lang.RuntimeException:
> java.lang.RuntimeException: java.io.NotSerializableException:
> org.apache.openejb.cdi.Proxys$ThreadLocalHandler
> 
> Looking at the Proxys.java code, it contains a private static class called
> ThreadLocalHandler, that is indeed not Serializable. For fun, I made
> ThreadLocalHandler implement Serializable, but that just resulted in
> another error because ThreadLocalHandler contains a ThreadLocal field which
> is definitely not Serializable.
> 
> ThreadLocalHandler for HttpServletRequest is invoked by
> TomcatWebappBuilder.setComponentsUsedByCDI():
> 
> https://github.com/apache/tomee/blob/tomee-8.x/tomee/tomee-catalina/src/main/java/org/apache/tomee/catalina/TomcatWebAppBuilder.java#L358
> 
> I'm at a bit of a loss on how to fix this. My assumption is the
> ThreadLocalHandler was designed as a generic proxy object that defers to an
> internal ThreadLocal object. I wonder if marking the ThreadLocal as
> transient is sufficient, but I could use some input.
> 
> Thank you,
> 
> -- 
> Jonathan | exabr...@gmail.com
> Pessimists, see a jar as half empty. Optimists, in contrast, see it as half
> full.
> Engineers, of course, understand the glass is twice as big as it needs to
> be.

Reply via email to