grails / grails-mail

The Grails Mail Plugin
https://grails.github.io/grails-mail/
Apache License 2.0
15 stars 25 forks source link

Grails 4 M1, OpenJDK 11.0.2 throws exception with basic usage #36

Closed erichelgeson closed 5 years ago

erichelgeson commented 5 years ago

Grails 4 M1 OpenJDK 11.0.2

Commit on a fresh 4.0.0.M1 project https://github.com/erichelgeson/grails4/commit/44bfdd583842680a918581a3514350572571b871

Hit /mail/index - get:

2019-02-19 14:56:11.064 ERROR --- [nio-8080-exec-1] o.g.web.errors.GrailsExceptionResolver   : IllegalArgumentException occurred when processing request: [GET] /mail/index
Stacktrace follows:

org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'four.MailController': Initialization of bean failed; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'mailService': Invocation of init method failed; nested exception is java.lang.IllegalArgumentException
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:601)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:515)
    at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:320)
    at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:222)
    at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:318)
    at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:199)
    at org.springframework.context.support.AbstractApplicationContext.getBean(AbstractApplicationContext.java:1105)
    at org.grails.web.mapping.mvc.UrlMappingsInfoHandlerAdapter.handle(UrlMappingsInfoHandlerAdapter.groovy:73)
    at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1038)
    at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:942)
    at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1005)
    at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:897)
    at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:882)
    at org.grails.web.servlet.mvc.GrailsWebRequestFilter.doFilterInternal(GrailsWebRequestFilter.java:77)
    at org.grails.web.filters.HiddenHttpMethodFilter.doFilterInternal(HiddenHttpMethodFilter.java:67)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at java.base/java.lang.Thread.run(Thread.java:834)
Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'mailService': Invocation of init method failed; nested exception is java.lang.IllegalArgumentException
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1762)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:593)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:515)
    at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:320)
    at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:222)
    at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:318)
    at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:199)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.autowireByName(AbstractAutowireCapableBeanFactory.java:1436)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.populateBean(AbstractAutowireCapableBeanFactory.java:1375)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:592)
    ... 17 common frames omitted
Caused by: java.lang.IllegalArgumentException: null
    at java.base/java.util.concurrent.ThreadPoolExecutor.setCorePoolSize(ThreadPoolExecutor.java:1535)
    at grails.plugins.mail.MailService.setPoolSize(MailService.groovy:66)
    at grails.plugins.mail.MailService.afterPropertiesSet(MailService.groovy:86)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.invokeInitMethods(AbstractAutowireCapableBeanFactory.java:1821)
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1758)
    ... 26 common frames omitted
drennane commented 5 years ago

I managed to temporarily circumvent this issue by adding the following to my application.groovy config

grails.mail.poolSize = 1

Seems the ThreadPoolExecutor no longer allows a maximumPoolSize smaller than the corePoolSize for which the plugin sets the former to 1 and the latter defaults to 5 if left undefined.

if (corePoolSize < 0 || maximumPoolSize < corePoolSize)
   throw new IllegalArgumentException();
swEngineer4ever commented 5 years ago

@drennane Did you have any side effects for your solution? :))

drennane commented 5 years ago

@swEngineer4ever I have not tested it to any great extent as of yet, but it is legitimate config (as opposed to being a hack) and so I would suggest that the answer is application dependent.

The config merely stipulates that a single thread will be made available for sending email. I guess if your application is busy and email is a large part of its functionality (and is time sensitive), you may end up with large queues being processed by a single thread.

In my application, it's quite sporadic and so I do not see a problem developing for me from the outset.

swEngineer4ever commented 5 years ago

@drennane Thanks for your quick reply. Is there a way to change the maximumPoolSize instead? Where this can be changed in the plugin?

drennane commented 5 years ago

@swEngineer4ever Funnily enough, the plugin actually sets the max size to the same as configured core size. The problem arises out of the order this is done because setting the core size is done first which in the updated implementation in Java's underlying ThreadPoolExecutor class, checks the max size (see the if statement above) and so bums out.

void setPoolSize(Integer poolSize){
   mailExecutorService.setCorePoolSize(poolSize ?: DEFAULT_POOL_SIZE)
   mailExecutorService.setMaximumPoolSize(poolSize ?: DEFAULT_POOL_SIZE)
}

The reason why setting to 1 works is because the initial constructor sets the max to 1 and so the assertion passes.

mailExecutorService = new ThreadPoolExecutor(1, 1, 60, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>());

I don't see a separate facility to set the max size and I'm not sure if meta programming would work in this instance because the class is annotated with @CompileStatic but someone can correct me if I'm wrong on that.

swEngineer4ever commented 5 years ago

@drennane exactly this is what I noticed also -> the plugin sets the max size the same as core size, that's why I was wondering from where the 1 comes :/ thanks for the explanation.

I changed the order of setCorePoolSize and setMaximumPoolSize in setPoolSize and the error is gone.

drennane commented 5 years ago

Cool. Might be worth a pull request.

HelainSchoonjans commented 5 years ago

Hey, seems you have a fix, when are you releasing a new version of the plugin?

erichelgeson commented 5 years ago

3.0.0 has been released.