From 33d655a63479ea6fdfda96b142e39bc5412f68ca Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Sat, 10 Feb 2018 13:03:33 +0100 Subject: [PATCH] Consistent handling of InterruptedException (plus setSchedulerFactory) Issue: SPR-16479 Issue: SPR-16439 (cherry picked from commit 39201ad) --- .../quartz/SchedulerFactoryBean.java | 159 +++++++++++------- .../LocalSessionFactoryBuilder.java | 6 +- .../jpa/AbstractEntityManagerFactoryBean.java | 6 +- .../test/web/servlet/DefaultMvcResult.java | 3 +- .../http/client/Netty4ClientHttpRequest.java | 5 +- .../sockjs/client/UndertowXhrTransport.java | 6 +- 6 files changed, 109 insertions(+), 76 deletions(-) diff --git a/spring-context-support/src/main/java/org/springframework/scheduling/quartz/SchedulerFactoryBean.java b/spring-context-support/src/main/java/org/springframework/scheduling/quartz/SchedulerFactoryBean.java index f53736e5213..a3fb2d1b965 100644 --- a/spring-context-support/src/main/java/org/springframework/scheduling/quartz/SchedulerFactoryBean.java +++ b/spring-context-support/src/main/java/org/springframework/scheduling/quartz/SchedulerFactoryBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -105,11 +105,10 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe new ThreadLocal(); /** - * Return the ResourceLoader for the currently configured Quartz Scheduler, - * to be used by ResourceLoaderClassLoadHelper. - *

This instance will be set before initialization of the corresponding - * Scheduler, and reset immediately afterwards. It is thus only available - * during configuration. + * Return the {@link ResourceLoader} for the currently configured Quartz Scheduler, + * to be used by {@link ResourceLoaderClassLoadHelper}. + *

This instance will be set before initialization of the corresponding Scheduler, + * and reset immediately afterwards. It is thus only available during configuration. * @see #setApplicationContext * @see ResourceLoaderClassLoadHelper */ @@ -118,11 +117,11 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe } /** - * Return the TaskExecutor for the currently configured Quartz Scheduler, - * to be used by LocalTaskExecutorThreadPool. - *

This instance will be set before initialization of the corresponding - * Scheduler, and reset immediately afterwards. It is thus only available - * during configuration. + * Return the {@link Executor} for the currently configured Quartz Scheduler, + * to be used by {@link LocalTaskExecutorThreadPool}. + *

This instance will be set before initialization of the corresponding Scheduler, + * and reset immediately afterwards. It is thus only available during configuration. + * @since 2.0 * @see #setTaskExecutor * @see LocalTaskExecutorThreadPool */ @@ -131,11 +130,11 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe } /** - * Return the DataSource for the currently configured Quartz Scheduler, - * to be used by LocalDataSourceJobStore. - *

This instance will be set before initialization of the corresponding - * Scheduler, and reset immediately afterwards. It is thus only available - * during configuration. + * Return the {@link DataSource} for the currently configured Quartz Scheduler, + * to be used by {@link LocalDataSourceJobStore}. + *

This instance will be set before initialization of the corresponding Scheduler, + * and reset immediately afterwards. It is thus only available during configuration. + * @since 1.1 * @see #setDataSource * @see LocalDataSourceJobStore */ @@ -144,11 +143,11 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe } /** - * Return the non-transactional DataSource for the currently configured - * Quartz Scheduler, to be used by LocalDataSourceJobStore. - *

This instance will be set before initialization of the corresponding - * Scheduler, and reset immediately afterwards. It is thus only available - * during configuration. + * Return the non-transactional {@link DataSource} for the currently configured + * Quartz Scheduler, to be used by {@link LocalDataSourceJobStore}. + *

This instance will be set before initialization of the corresponding Scheduler, + * and reset immediately afterwards. It is thus only available during configuration. + * @since 1.1 * @see #setNonTransactionalDataSource * @see LocalDataSourceJobStore */ @@ -157,6 +156,8 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe } + private SchedulerFactory schedulerFactory; + private Class schedulerFactoryClass = StdSchedulerFactory.class; private String schedulerName; @@ -165,14 +166,12 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe private Properties quartzProperties; - private Executor taskExecutor; private DataSource dataSource; private DataSource nonTransactionalDataSource; - private Map schedulerContextMap; private ApplicationContext applicationContext; @@ -183,7 +182,6 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe private boolean jobFactorySet = false; - private boolean autoStartup = true; private int startupDelay = 0; @@ -194,19 +192,38 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe private boolean waitForJobsToCompleteOnShutdown = false; - private Scheduler scheduler; /** - * Set the Quartz SchedulerFactory implementation to use. - *

Default is {@link StdSchedulerFactory}, reading in the standard - * {@code quartz.properties} from {@code quartz.jar}. - * To use custom Quartz properties, specify the "configLocation" - * or "quartzProperties" bean property on this FactoryBean. + * Set an external Quartz {@link SchedulerFactory} instance to use. + *

Default is an internal {@link StdSchedulerFactory} instance. If this method is + * called, it overrides any class specified through {@link #setSchedulerFactoryClass} + * as well as any settings specified through {@link #setConfigLocation}, + * {@link #setQuartzProperties}, {@link #setTaskExecutor} or {@link #setDataSource}. + *

NOTE: With an externally provided {@code SchedulerFactory} instance, + * local settings such as {@link #setConfigLocation} or {@link #setQuartzProperties} + * will be ignored here in {@code SchedulerFactoryBean}, expecting the external + * {@code SchedulerFactory} instance to get initialized on its own. + * @since 4.3.15 + * @see #setSchedulerFactoryClass + */ + public void setSchedulerFactory(SchedulerFactory schedulerFactory) { + this.schedulerFactory = schedulerFactory; + } + + /** + * Set the Quartz {@link SchedulerFactory} implementation to use. + *

Default is the {@link StdSchedulerFactory} class, reading in the standard + * {@code quartz.properties} from {@code quartz.jar}. For applying custom Quartz + * properties, specify {@link #setConfigLocation "configLocation"} and/or + * {@link #setQuartzProperties "quartzProperties"} etc on this local + * {@code SchedulerFactoryBean} instance. * @see org.quartz.impl.StdSchedulerFactory * @see #setConfigLocation * @see #setQuartzProperties + * @see #setTaskExecutor + * @see #setDataSource */ public void setSchedulerFactoryClass(Class schedulerFactoryClass) { this.schedulerFactoryClass = schedulerFactoryClass; @@ -244,14 +261,14 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe this.quartzProperties = quartzProperties; } - /** - * Set the Spring TaskExecutor to use as Quartz backend. + * Set a Spring-managed {@link Executor} to use as Quartz backend. * Exposed as thread pool through the Quartz SPI. - *

Can be used to assign a JDK 1.5 ThreadPoolExecutor or a CommonJ + *

Can be used to assign a local JDK ThreadPoolExecutor or a CommonJ * WorkManager as Quartz backend, to avoid Quartz's manual thread creation. *

By default, a Quartz SimpleThreadPool will be used, configured through * the corresponding Quartz properties. + * @since 2.0 * @see #setQuartzProperties * @see LocalTaskExecutorThreadPool * @see org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor @@ -262,8 +279,8 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe } /** - * Set the default DataSource to be used by the Scheduler. If set, - * this will override corresponding settings in Quartz properties. + * Set the default {@link DataSource} to be used by the Scheduler. + * If set, this will override corresponding settings in Quartz properties. *

Note: If this is set, the Quartz settings should not define * a job store "dataSource" to avoid meaningless double configuration. *

A Spring-specific subclass of Quartz' JobStoreCMT will be used. @@ -276,6 +293,7 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe * argument is sufficient. In case of an XA DataSource and global JTA transactions, * SchedulerFactoryBean's "nonTransactionalDataSource" property should be set, * passing in a non-XA DataSource that will not participate in global transactions. + * @since 1.1 * @see #setNonTransactionalDataSource * @see #setQuartzProperties * @see #setTransactionManager @@ -286,12 +304,13 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe } /** - * Set the DataSource to be used by the Scheduler for non-transactional access. + * Set the {@link DataSource} to be used for non-transactional access. *

This is only necessary if the default DataSource is an XA DataSource that will * always participate in transactions: A non-XA version of that DataSource should * be specified as "nonTransactionalDataSource" in such a scenario. *

This is not relevant with a local DataSource instance and Spring transactions. * Specifying a single default DataSource as "dataSource" is sufficient there. + * @since 1.1 * @see #setDataSource * @see LocalDataSourceJobStore */ @@ -299,7 +318,6 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe this.nonTransactionalDataSource = nonTransactionalDataSource; } - /** * Register objects in the Scheduler context via a given Map. * These objects will be available to any Job that runs in this Scheduler. @@ -315,7 +333,7 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe } /** - * Set the key of an ApplicationContext reference to expose in the + * Set the key of an {@link ApplicationContext} reference to expose in the * SchedulerContext, for example "applicationContext". Default is none. * Only applicable when running in a Spring ApplicationContext. *

Note: When using persistent Jobs whose JobDetail will be kept in the @@ -335,7 +353,7 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe } /** - * Set the Quartz JobFactory to use for this Scheduler. + * Set the Quartz {@link JobFactory} to use for this Scheduler. *

Default is Spring's {@link AdaptableJobFactory}, which supports * {@link java.lang.Runnable} objects as well as standard Quartz * {@link org.quartz.Job} instances. Note that this default only applies @@ -344,6 +362,7 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe *

Specify an instance of Spring's {@link SpringBeanJobFactory} here * (typically as an inner bean definition) to automatically populate a job's * bean properties from the specified job data map and scheduler context. + * @since 2.0 * @see AdaptableJobFactory * @see SpringBeanJobFactory */ @@ -352,7 +371,6 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe this.jobFactorySet = true; } - /** * Set whether to automatically start the scheduler after initialization. *

Default is "true"; set this to "false" to allow for manual startup. @@ -372,11 +390,12 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe } /** - * Specify the phase in which this scheduler should be started and - * stopped. The startup order proceeds from lowest to highest, and - * the shutdown order is the reverse of that. By default this value - * is Integer.MAX_VALUE meaning that this scheduler starts as late - * as possible and stops as soon as possible. + * Specify the phase in which this scheduler should be started and stopped. + * The startup order proceeds from lowest to highest, and the shutdown order + * is the reverse of that. By default this value is {@code Integer.MAX_VALUE} + * meaning that this scheduler starts as late as possible and stops as soon + * as possible. + * @since 3.0 */ public void setPhase(int phase) { this.phase = phase; @@ -424,7 +443,6 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe this.waitForJobsToCompleteOnShutdown = waitForJobsToCompleteOnShutdown; } - @Override public void setBeanName(String name) { if (this.schedulerName == null) { @@ -452,9 +470,8 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe this.resourceLoader = this.applicationContext; } - // Create SchedulerFactory instance... - SchedulerFactory schedulerFactory = BeanUtils.instantiateClass(this.schedulerFactoryClass); - initSchedulerFactory(schedulerFactory); + // Initialize the SchedulerFactory instance... + SchedulerFactory schedulerFactory = prepareSchedulerFactory(); if (this.resourceLoader != null) { // Make given ResourceLoader available for SchedulerFactory configuration. @@ -512,22 +529,34 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe /** - * Load and/or apply Quartz properties to the given SchedulerFactory. - * @param schedulerFactory the SchedulerFactory to initialize + * Create a SchedulerFactory if necessary and apply locally defined Quartz properties to it. + * @return the initialized SchedulerFactory */ - private void initSchedulerFactory(SchedulerFactory schedulerFactory) throws SchedulerException, IOException { - if (!(schedulerFactory instanceof StdSchedulerFactory)) { - if (this.configLocation != null || this.quartzProperties != null || + private SchedulerFactory prepareSchedulerFactory() throws SchedulerException, IOException { + SchedulerFactory schedulerFactory = this.schedulerFactory; + if (schedulerFactory == null) { + // Create local SchedulerFactory instance (typically a StdSchedulerFactory) + schedulerFactory = BeanUtils.instantiateClass(this.schedulerFactoryClass); + if (schedulerFactory instanceof StdSchedulerFactory) { + initSchedulerFactory((StdSchedulerFactory) schedulerFactory); + } + else if (this.configLocation != null || this.quartzProperties != null || this.taskExecutor != null || this.dataSource != null) { throw new IllegalArgumentException( "StdSchedulerFactory required for applying Quartz properties: " + schedulerFactory); } - // Otherwise assume that no initialization is necessary... - return; + // Otherwise, no local settings to be applied via StdSchedulerFactory.initialize(Properties) } + // Otherwise, assume that externally provided factory has been initialized with appropriate settings + return schedulerFactory; + } + /** + * Initialize the given SchedulerFactory, applying locally defined Quartz properties to it. + * @param schedulerFactory the SchedulerFactory to initialize + */ + private void initSchedulerFactory(StdSchedulerFactory schedulerFactory) throws SchedulerException, IOException { Properties mergedProps = new Properties(); - if (this.resourceLoader != null) { mergedProps.setProperty(StdSchedulerFactory.PROP_SCHED_CLASS_LOAD_HELPER_CLASS, ResourceLoaderClassLoadHelper.class.getName()); @@ -552,17 +581,14 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe } CollectionUtils.mergePropertiesIntoMap(this.quartzProperties, mergedProps); - if (this.dataSource != null) { mergedProps.put(StdSchedulerFactory.PROP_JOB_STORE_CLASS, LocalDataSourceJobStore.class.getName()); } - - // Make sure to set the scheduler name as configured in the Spring configuration. if (this.schedulerName != null) { mergedProps.put(StdSchedulerFactory.PROP_SCHED_INSTANCE_NAME, this.schedulerName); } - ((StdSchedulerFactory) schedulerFactory).initialize(mergedProps); + schedulerFactory.initialize(mergedProps); } /** @@ -619,7 +645,7 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe private void populateSchedulerContext() throws SchedulerException { // Put specified objects into Scheduler context. if (this.schedulerContextMap != null) { - this.scheduler.getContext().putAll(this.schedulerContextMap); + getScheduler().getContext().putAll(this.schedulerContextMap); } // Register ApplicationContext in Scheduler context. @@ -629,7 +655,7 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe "SchedulerFactoryBean needs to be set up in an ApplicationContext " + "to be able to handle an 'applicationContextSchedulerContextKey'"); } - this.scheduler.getContext().put(this.applicationContextSchedulerContextKey, this.applicationContext); + getScheduler().getContext().put(this.applicationContextSchedulerContextKey, this.applicationContext); } } @@ -659,6 +685,7 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe Thread.sleep(startupDelay * 1000); } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); // simply proceed } if (logger.isInfoEnabled()) { @@ -762,8 +789,10 @@ public class SchedulerFactoryBean extends SchedulerAccessor implements FactoryBe */ @Override public void destroy() throws SchedulerException { - logger.info("Shutting down Quartz Scheduler"); - this.scheduler.shutdown(this.waitForJobsToCompleteOnShutdown); + if (this.scheduler != null) { + logger.info("Shutting down Quartz Scheduler"); + this.scheduler.shutdown(this.waitForJobsToCompleteOnShutdown); + } } } diff --git a/spring-orm-hibernate5/src/main/java/org/springframework/orm/hibernate5/LocalSessionFactoryBuilder.java b/spring-orm-hibernate5/src/main/java/org/springframework/orm/hibernate5/LocalSessionFactoryBuilder.java index cdace8ec263..fd08c996cb7 100644 --- a/spring-orm-hibernate5/src/main/java/org/springframework/orm/hibernate5/LocalSessionFactoryBuilder.java +++ b/spring-orm-hibernate5/src/main/java/org/springframework/orm/hibernate5/LocalSessionFactoryBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -423,8 +423,8 @@ public class LocalSessionFactoryBuilder extends Configuration { return this.sessionFactoryFuture.get(); } catch (InterruptedException ex) { - throw new IllegalStateException("Interrupted during initialization of Hibernate SessionFactory: " + - ex.getMessage()); + Thread.currentThread().interrupt(); + throw new IllegalStateException("Interrupted during initialization of Hibernate SessionFactory", ex); } catch (ExecutionException ex) { throw new IllegalStateException("Failed to asynchronously initialize Hibernate SessionFactory: " + diff --git a/spring-orm/src/main/java/org/springframework/orm/jpa/AbstractEntityManagerFactoryBean.java b/spring-orm/src/main/java/org/springframework/orm/jpa/AbstractEntityManagerFactoryBean.java index b6d008c8d40..3c672e25a18 100644 --- a/spring-orm/src/main/java/org/springframework/orm/jpa/AbstractEntityManagerFactoryBean.java +++ b/spring-orm/src/main/java/org/springframework/orm/jpa/AbstractEntityManagerFactoryBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -514,8 +514,8 @@ public abstract class AbstractEntityManagerFactoryBean implements return this.nativeEntityManagerFactoryFuture.get(); } catch (InterruptedException ex) { - throw new IllegalStateException("Interrupted during initialization of native EntityManagerFactory: " + - ex.getMessage()); + Thread.currentThread().interrupt(); + throw new IllegalStateException("Interrupted during initialization of native EntityManagerFactory", ex); } catch (ExecutionException ex) { throw new IllegalStateException("Failed to asynchronously initialize native EntityManagerFactory: " + diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/DefaultMvcResult.java b/spring-test/src/main/java/org/springframework/test/web/servlet/DefaultMvcResult.java index 4bf5f8dd10f..2273388fbf1 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/DefaultMvcResult.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/DefaultMvcResult.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -134,6 +134,7 @@ class DefaultMvcResult implements MvcResult { Thread.sleep(100); } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); throw new IllegalStateException("Interrupted while waiting for " + "async result to be set for handler [" + this.handler + "]", ex); } diff --git a/spring-web/src/main/java/org/springframework/http/client/Netty4ClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/Netty4ClientHttpRequest.java index 9acfb45efa2..05d4a54f4ef 100644 --- a/spring-web/src/main/java/org/springframework/http/client/Netty4ClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/Netty4ClientHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -86,7 +86,8 @@ class Netty4ClientHttpRequest extends AbstractAsyncClientHttpRequest implements return executeAsync().get(); } catch (InterruptedException ex) { - throw new IOException(ex.getMessage(), ex); + Thread.currentThread().interrupt(); + throw new IOException("Interrupted during request execution", ex); } catch (ExecutionException ex) { if (ex.getCause() instanceof IOException) { diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/UndertowXhrTransport.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/UndertowXhrTransport.java index 4f70016a12d..a31e70a47d4 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/UndertowXhrTransport.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/UndertowXhrTransport.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -57,6 +57,7 @@ import org.springframework.http.ResponseEntity; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; +import org.springframework.util.StringUtils; import org.springframework.util.concurrent.SettableListenableFuture; import org.springframework.web.client.HttpServerErrorException; import org.springframework.web.socket.CloseStatus; @@ -281,7 +282,7 @@ public class UndertowXhrTransport extends AbstractXhrTransport { try { ClientRequest request = new ClientRequest().setMethod(method).setPath(url.getPath()); request.getRequestHeaders().add(HttpString.tryFromString(HttpHeaders.HOST), url.getHost()); - if (body != null && !body.isEmpty()) { + if (StringUtils.hasLength(body)) { HttpString headerName = HttpString.tryFromString(HttpHeaders.CONTENT_LENGTH); request.getRequestHeaders().add(headerName, body.length()); } @@ -305,6 +306,7 @@ public class UndertowXhrTransport extends AbstractXhrTransport { throw new SockJsTransportFailureException("Failed to execute request to " + url, ex); } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); throw new SockJsTransportFailureException("Interrupted while processing request to " + url, ex); } }