Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ApplicationListenerDetector should prevent serialization of its ApplicationContext reference [SPR-14214] #18788

Closed
spring-issuemaster opened this issue Apr 25, 2016 · 3 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Apr 25, 2016

Ricardo Fanjul Fandiño opened SPR-14214 and commented

I'm trying to migrate Spring Web Application from 3.x version to 4.2 version of Spring.

My application is deployed in a cluster of Tomcat 7 using for serialize the session: memcached-session-manager: https://github.com/magro/memcached-session-manager and serializing the objects with Kryo.

When serialize the session I found a mistake that made me suspect that I'm trying to serialize the Spring ApplicationContext.

Debugging my application I found this:
!imagen1.png|thumbnail!

Inside the session exist a key “org.springframework.web.context.request.ServletRequestAttributes.DESTRUCTION_CALLBACK.scopedTarget.restSessionDataHolder” whose value reference the ApplicationContext.

I find that, for each bean declared in Session Scope. For example:

@Component
@Scope(proxyMode=ScopedProxyMode.TARGET_CLASS,value="session")
public class RestSessionDataHolder implements Serializable{
...

Spring in the method “org.springframework.web.context.request.ServletRequestAttributes.registerSessionDestructionCallback
(String name, Runnable callback)”, store in the session a key named “org.springframework.web.context.request.ServletRequestAttributes.DESTRUCTION_CALLBACK.scopedTarget.[BEAN_NAME]” with a value that indirectly reference the ApplicatonContext.

!imagen2.png|thumbnail!

!imagen3.png|thumbnail!

Inside this atribute exist two “DestructionAwareBeanPostProcessor”: “CommonAnnotationBeanPostProcessor” and “org.springframework.context.support.PostProcessorRegistrationDelegate$ApplicationListenerDetector”.

“PostProcessorRegistrationDelegate$ApplicationListenerDetector” exist since Spring 4.0 and maybe have a bug:

private static class ApplicationListenerDetector implements MergedBeanDefinitionPostProcessor, DestructionAwareBeanPostProcessor {
...
		private final AbstractApplicationContext applicationContext;

I think that attribute “private final AbstractApplicationContext applicationContext” should be “transient”.

For example the similar attributes of “CommonAnnotationBeanPostProcessor” are transient:

public class CommonAnnotationBeanPostProcessor extends InitDestroyAnnotationBeanPostProcessor
		implements InstantiationAwareBeanPostProcessor, BeanFactoryAware, Serializable {

...

	private transient BeanFactory jndiFactory = new SimpleJndiBeanFactory();

	private transient BeanFactory resourceFactory;

	private transient BeanFactory beanFactory;

	private transient final Map<String, InjectionMetadata> injectionMetadataCache =
			new ConcurrentHashMap<String, InjectionMetadata>(256);

Affects: 4.1.7, 4.1.8, 4.1.9, 4.2 GA, 4.2.1, 4.2.2, 4.2.3, 4.2.4, 4.2.5

Attachments:

Issue Links:

  • #18317 Avoid scoped destruction callbacks in case of no post-processor actually applying
  • #19349 Inner bean behind BeanFactoryPostProcessor should be able to receive application events

Referenced from: commits 75a8f5b, e0734ae

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 27, 2016

Juergen Hoeller commented

Spring's DisposableBeanAdapter has a writeReplace implementation which filters for serializable post-processors... Since ApplicationListenerDetector isn't marked as serializable, it won't be included to begin with. However, that's only for standard Java serialization which actually respects writeReplace... Could this have to do with your specific arrangement, trying to serialize the original DisposableBeanAdapter instance and not its replacement object?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 27, 2016

Juergen Hoeller commented

Note that, as of Spring Framework 4.3, we do not create scoped destruction callbacks at all anymore if none of them has anything to do. So ApplicationListenerDetector won't be included for non-listener beans anymore.

That said, it nevertheless doesn't hurt to mark ApplicationListenerDetector's field state as transient, even if this might only really help with alternative serialization mechanisms. We can also easily backport that part to 4.2.6, whereas the above mechanism is 4.3 only.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 28, 2016

Ricardo Fanjul Fandiño commented

Thanks for change, solves my problem.

Then, I tried the "writeReplace()" and I found that Kryo don't invoke the method "writeReplace()".

"memcached-session-manager" have many serialization strategies:
https://github.com/magro/memcached-session-manager/wiki/SerializationStrategies.

I tried (Java serialization):
"de.javakaffee.web.msm.JavaSerializationTranscoderFactory".

And "DisposableBeanAdapter.writeReplace()" is invoked and "ApplicationListenerDetector" not included.

The problema is with "Kryo" serialization:
"de.javakaffee.web.msm.serializer.kryo.KryoTranscoderFactory"

"DisposableBeanAdapter.writeReplace()" is not invoked so, the "ApplicationListenerDetector" is included.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.