gaojunda / simple-spring-memcached

Automatically exported from code.google.com/p/simple-spring-memcached
MIT License
0 stars 0 forks source link

com.google.code.ssm.CacheFactory should be a DisposableBean to permit tomcat shutdown ? #6

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
With simple-spring-memcached and spymemcached backend, tomcat (7.0.27) shutdown 
timeouts. Error logs indicates that MemcacheClient thread cannot be stopped.

I see that net.spy.memcached.MemcachedClient implements a shutdown method, but 
it is not called during the shutdown process.

If I extend CacheFactory with a destroy method (that call cache.shutdown()) and 
implements DisposableBean, then the MemcachedClient.shutdown() method is called 
and tomcat can stop.

Should this issue be fixed in simple-spring-memcached ?

Original issue reported on code.google.com by lalme...@gmail.com on 6 Aug 2012 at 2:13

GoogleCodeExporter commented 8 years ago
Which version of SSM do you use?

MemcacheClient thread should be stopped on server shoutdown and to achieve this 
com.google.code.ssm.CacheImpl has shutdown method which invokes shutdown on 
client (in your case on spymemcached). This method in CacheImpl is annotated 
with @PreDestroy so it should be invoked when container want to remove instance 
of this class. 
Could you confirm that this method is not invoked and @PreDestroy doesn't work 
as  expected?

Original comment by ragno...@gmail.com on 6 Aug 2012 at 6:02

GoogleCodeExporter commented 8 years ago
I use ssm 3.0.1

I confirm that CacheImpl.shutdown() is not called on server shutdown without my 
workaround (so that @PreDestroy is not called).

I can see two reasons for that :
 * CacheImpl is built from CacheFactory (which is a FactoryBean<Cache>) and @PreDestroy does not apply on bean built by FactoryBean ?
 * component-scan is not enabled for ssm package so the @PreDestroy annotation is ignored ?

Original comment by lalme...@gmail.com on 6 Aug 2012 at 6:34

GoogleCodeExporter commented 8 years ago
The component-scan is not the reason because even if CacheImpl implements 
DisposableBean the destroy method is not called. Could you check this on tomcat?
It seams that bean created by factory (FactoryBean) doesn't have usual spring 
bean lifecycle and the factory is responsible for destroying it.

Original comment by ragno...@gmail.com on 6 Aug 2012 at 9:15

GoogleCodeExporter commented 8 years ago
What do you want I check with tomcat ?

Original comment by lalme...@gmail.com on 6 Aug 2012 at 9:58

GoogleCodeExporter commented 8 years ago
Implement in CacheImpl DisposableBean and in destroy method call shutdown():

class CacheImpl implements Cache, DisposableBean {
 .....
    @Override
    public void destroy() throws Exception {
        shutdown();        
    }
}

I want to make sure that this doesn't work on tomcat before I make factory 
responsible for destroying cache.

Original comment by ragno...@gmail.com on 7 Aug 2012 at 5:16

GoogleCodeExporter commented 8 years ago
I override CacheImpl implementation with a copy that implements DisposableBean 
and destroy method is not called.

It seems consistent with this blog post [1] (last paragraph) that states that 
beans issued from FactoryBean are not managed by spring. It seems also 
consistent that @PreDestroy and DisposableBean#destroy are called (or not 
called) in the same condition.

The only way to fix that may be to implements DisposableBean (or @PreDestroy) 
in CacheFactory.

[1] http://blog.springsource.org/2011/08/09/whats-a-factorybean/

Original comment by lalme...@gmail.com on 7 Aug 2012 at 9:51

GoogleCodeExporter commented 8 years ago
Thanks for the reference to blog post and explanation.

Fix is available in trunk and will be released with 3.0.2. I hope I'll find 
time to do it today.

Original comment by ragno...@gmail.com on 7 Aug 2012 at 10:22

GoogleCodeExporter commented 8 years ago
Version 3.0.2 has been released and deployed to central maven repository.

Original comment by ragno...@gmail.com on 8 Aug 2012 at 5:27

GoogleCodeExporter commented 8 years ago
I'm a bit late, but I confirm 3.0.2 fixes my problem. Thanks for the fix.

Original comment by lalme...@gmail.com on 29 Aug 2012 at 5:11

GoogleCodeExporter commented 8 years ago
Thank you for the confirmation.

Original comment by ragno...@gmail.com on 29 Aug 2012 at 5:18