Shutdown executor during destroy()
The threads in the Executor are non-daemon, and thus will prevent
graceful shutdown of the Java process. In addition, after destroy(),
nothing should be accessing the AdaptorContext, which is not necessarily
the case when we are sending feeds on other threads.
diff --git a/src/com/google/enterprise/adaptor/sharepoint/SharePointAdaptor.java b/src/com/google/enterprise/adaptor/sharepoint/SharePointAdaptor.java
index 2b4c6c6..92d3417 100644
--- a/src/com/google/enterprise/adaptor/sharepoint/SharePointAdaptor.java
+++ b/src/com/google/enterprise/adaptor/sharepoint/SharePointAdaptor.java
@@ -266,7 +266,8 @@
private final UserGroupFactory userGroupFactory;
/** Client for initiating raw HTTP connections. */
private final HttpClient httpClient;
- private final Executor executor;
+ private final Callable<ExecutorService> executorFactory;
+ private ExecutorService executor;
private boolean xmlValidation;
/** Authenticator instance that authenticates with SP. */
/**
@@ -277,20 +278,21 @@
public SharePointAdaptor() {
this(new SiteDataFactoryImpl(), new UserGroupFactoryImpl(),
- new HttpClientImpl(), Executors.newCachedThreadPool());
+ new HttpClientImpl(), new CachedThreadPoolFactory());
}
@VisibleForTesting
SharePointAdaptor(SiteDataFactory siteDataFactory,
- UserGroupFactory userGroupFactory, HttpClient httpClient, Executor executor) {
+ UserGroupFactory userGroupFactory, HttpClient httpClient,
+ Callable<ExecutorService> executorFactory) {
if (siteDataFactory == null || httpClient == null
- || userGroupFactory == null || executor == null) {
+ || userGroupFactory == null || executorFactory == null) {
throw new NullPointerException();
}
this.siteDataFactory = siteDataFactory;
this.userGroupFactory = userGroupFactory;
this.httpClient = httpClient;
- this.executor = executor;
+ this.executorFactory = executorFactory;
}
/**
@@ -316,7 +318,7 @@
}
@Override
- public void init(AdaptorContext context) throws IOException {
+ public void init(AdaptorContext context) throws Exception {
this.context = context;
Config config = context.getConfig();
virtualServer = config.getValue("sharepoint.server");
@@ -335,11 +337,21 @@
virtualServerUrl.getHost(), virtualServerUrl.getPort());
// Unfortunately, this is a JVM-wide modification.
Authenticator.setDefault(ntlmAuthenticator);
+
+ executor = executorFactory.call();
}
@Override
public void destroy() {
Authenticator.setDefault(null);
+ executor.shutdown();
+ try {
+ executor.awaitTermination(10, TimeUnit.SECONDS);
+ } catch (InterruptedException ex) {
+ Thread.currentThread().interrupt();
+ }
+ executor.shutdownNow();
+ executor = null;
}
@Override
@@ -2231,4 +2243,12 @@
return getSiteDataClient(site, site).retrieveSiteUserMapping();
}
}
+
+ private static class CachedThreadPoolFactory
+ implements Callable<ExecutorService> {
+ @Override
+ public ExecutorService call() {
+ return Executors.newCachedThreadPool();
+ }
+ }
}
diff --git a/test/com/google/enterprise/adaptor/sharepoint/CallerRunsExecutor.java b/test/com/google/enterprise/adaptor/sharepoint/CallerRunsExecutor.java
new file mode 100644
index 0000000..ef4caa3
--- /dev/null
+++ b/test/com/google/enterprise/adaptor/sharepoint/CallerRunsExecutor.java
@@ -0,0 +1,62 @@
+// Copyright 2013 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.enterprise.adaptor.sharepoint;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.*;
+
+/**
+ * A simple ExecutorService that executes all tasks in the calling thread. It
+ * does not implement termination semantics properly; it is immediately
+ * "terminated" after being shutdown, even if execute is still running.
+ */
+class CallerRunsExecutor extends AbstractExecutorService {
+ private volatile boolean isShutdown;
+
+ @Override
+ public void execute(Runnable command) {
+ if (isShutdown) {
+ throw new RejectedExecutionException("Executor already shutdown");
+ }
+ command.run();
+ }
+
+ @Override
+ public boolean awaitTermination(long timeout, TimeUnit unit) {
+ return isShutdown;
+ }
+
+ @Override
+ public boolean isShutdown() {
+ return isShutdown;
+ }
+
+ @Override
+ public boolean isTerminated() {
+ return isShutdown;
+ }
+
+ @Override
+ public void shutdown() {
+ isShutdown = true;
+ }
+
+ @Override
+ public List<Runnable> shutdownNow() {
+ shutdown();
+ return Collections.emptyList();
+ }
+}
diff --git a/test/com/google/enterprise/adaptor/sharepoint/SharePointAdaptorTest.java b/test/com/google/enterprise/adaptor/sharepoint/SharePointAdaptorTest.java
index 2055786..1fcc0c0 100644
--- a/test/com/google/enterprise/adaptor/sharepoint/SharePointAdaptorTest.java
+++ b/test/com/google/enterprise/adaptor/sharepoint/SharePointAdaptorTest.java
@@ -102,7 +102,13 @@
private final Charset charset = Charset.forName("UTF-8");
private Config config;
private SharePointAdaptor adaptor;
- private Executor executor = new UnsupportedExecutor();
+ private Callable<ExecutorService> executorFactory
+ = new Callable<ExecutorService>() {
+ @Override
+ public ExecutorService call() {
+ return new CallerRunsExecutor();
+ }
+ };
@Rule
public ExpectedException thrown = ExpectedException.none();
@@ -174,42 +180,42 @@
public void testNullSiteDataFactory() {
thrown.expect(NullPointerException.class);
new SharePointAdaptor(null, new UnsupportedUserGroupFactory(),
- new UnsupportedHttpClient(), executor);
+ new UnsupportedHttpClient(), executorFactory);
}
@Test
public void testNullUserGroupFactory() {
thrown.expect(NullPointerException.class);
new SharePointAdaptor(new UnsupportedSiteDataFactory(), null,
- new UnsupportedHttpClient(), executor);
+ new UnsupportedHttpClient(), executorFactory);
}
@Test
public void testNullHttpClient() {
thrown.expect(NullPointerException.class);
new SharePointAdaptor(new UnsupportedSiteDataFactory(),
- new UnsupportedUserGroupFactory(), null, executor);
+ new UnsupportedUserGroupFactory(), null, executorFactory);
}
-
+
@Test
- public void testNullExecutor() {
+ public void testNullExecutorFactory() {
thrown.expect(NullPointerException.class);
new SharePointAdaptor(new UnsupportedSiteDataFactory(),
new UnsupportedUserGroupFactory(), new UnsupportedHttpClient(), null);
}
@Test
- public void testInitDestroy() throws IOException {
+ public void testInitDestroy() throws Exception {
adaptor = new SharePointAdaptor(new UnsupportedSiteDataFactory(),
new UnsupportedUserGroupFactory(),
- new UnsupportedHttpClient(), executor);
+ new UnsupportedHttpClient(), executorFactory);
adaptor.init(new MockAdaptorContext(config, null));
adaptor.destroy();
adaptor = null;
}
@Test
- public void testGetDocContentWrongServer() throws IOException {
+ public void testGetDocContentWrongServer() throws Exception {
class WrongServerSiteData extends UnsupportedSiteData {
@Override
public void getSiteAndWeb(String strUrl, Holder<Long> getSiteAndWebResult,
@@ -226,7 +232,7 @@
new SingleSiteDataFactory(new WrongServerSiteData(),
"http://localhost:1/_vti_bin/SiteData.asmx"),
new UnsupportedUserGroupFactory(),
- new UnsupportedHttpClient(), executor);
+ new UnsupportedHttpClient(), executorFactory);
adaptor.init(new MockAdaptorContext(config, null));
ByteArrayOutputStream baos = new ByteArrayOutputStream();
GetContentsRequest request = new GetContentsRequest(
@@ -237,7 +243,7 @@
}
@Test
- public void testGetDocContentWrongPage() throws IOException {
+ public void testGetDocContentWrongPage() throws Exception {
final String wrongPage = "http://localhost:1/wrongPage";
class WrongPageSiteData extends UnsupportedSiteData {
@Override
@@ -269,7 +275,7 @@
new SingleSiteDataFactory(new WrongPageSiteData(),
"http://localhost:1/_vti_bin/SiteData.asmx"),
new UnsupportedUserGroupFactory(), new UnsupportedHttpClient(),
- executor);
+ executorFactory);
adaptor.init(new MockAdaptorContext(config, null));
ByteArrayOutputStream baos = new ByteArrayOutputStream();
GetContentsRequest request = new GetContentsRequest(new DocId(wrongPage));
@@ -279,7 +285,7 @@
}
@Test
- public void testGetDocContentVirtualServer() throws IOException {
+ public void testGetDocContentVirtualServer() throws Exception {
final String getContentVirtualServer
= loadTestString("vs.xml");
final String getContentContentDatabase
@@ -311,7 +317,7 @@
new SingleSiteDataFactory(new VirtualServerSiteData(),
"http://localhost:1/_vti_bin/SiteData.asmx"),
new UnsupportedUserGroupFactory(), new UnsupportedHttpClient(),
- executor);
+ executorFactory);
adaptor.init(new MockAdaptorContext(config, null));
ByteArrayOutputStream baos = new ByteArrayOutputStream();
GetContentsResponse response = new GetContentsResponse(baos);
@@ -339,7 +345,7 @@
}
@Test
- public void testGetDocContentSiteCollection() throws IOException {
+ public void testGetDocContentSiteCollection() throws Exception {
final String getContentSiteCollection
= loadTestString("sites-SiteCollection-sc.xml");
final String getContentSite
@@ -406,7 +412,7 @@
return new SiteCollectionSiteData(endpoint);
}
}, new UnsupportedUserGroupFactory(), new UnsupportedHttpClient(),
- executor);
+ executorFactory);
adaptor.init(new MockAdaptorContext(config, null));
ByteArrayOutputStream baos = new ByteArrayOutputStream();
GetContentsRequest request = new GetContentsRequest(
@@ -448,7 +454,7 @@
}
@Test
- public void testGetDocContentList() throws IOException {
+ public void testGetDocContentList() throws Exception {
final String getContentListResponse
= loadTestString("sites-SiteCollection-Lists-CustomList-l.xml");
final String getContentSite
@@ -515,7 +521,7 @@
adaptor = new SharePointAdaptor(new UnsupportedSiteDataFactory(),
new UnsupportedUserGroupFactory(), new UnsupportedHttpClient(),
- executor);
+ executorFactory);
adaptor.init(new MockAdaptorContext(config, null));
ByteArrayOutputStream baos = new ByteArrayOutputStream();
GetContentsRequest request = new GetContentsRequest(
@@ -550,7 +556,7 @@
}
@Test
- public void testGetDocContentAttachment() throws IOException {
+ public void testGetDocContentAttachment() throws Exception {
final String site = "http://localhost:1/sites/SiteCollection";
final String attachmentId = site + "/Lists/Custom List/Attachments/2/104600"
+ "0.pdf";
@@ -624,7 +630,7 @@
"conTent-TypE", goldenContentType, "Content-Type", "late");
return new FileInfo.Builder(contents).setHeaders(headers).build();
}
- }, executor);
+ }, executorFactory);
adaptor.init(new MockAdaptorContext(config, null));
ByteArrayOutputStream baos = new ByteArrayOutputStream();
GetContentsRequest request = new GetContentsRequest(
@@ -652,7 +658,7 @@
}
@Test
- public void testGetDocContentListItem() throws IOException {
+ public void testGetDocContentListItem() throws Exception {
final String getContentListResponse
= loadTestString("sites-SiteCollection-Lists-CustomList-l.xml");
final String getContentListItemResponse
@@ -731,7 +737,7 @@
adaptor = new SharePointAdaptor(new UnsupportedSiteDataFactory(),
new UnsupportedUserGroupFactory(), new UnsupportedHttpClient(),
- executor);
+ executorFactory);
adaptor.init(new MockAdaptorContext(config, null));
ByteArrayOutputStream baos = new ByteArrayOutputStream();
GetContentsRequest request = new GetContentsRequest(
@@ -811,7 +817,7 @@
}
@Test
- public void testGetDocContentListItemAnonymousAccess() throws IOException {
+ public void testGetDocContentListItemAnonymousAccess() throws Exception {
final String getContentListResponse
= loadTestString("sites-SiteCollection-Lists-CustomList-l.xml")
.replace(
@@ -903,7 +909,7 @@
adaptor = new SharePointAdaptor(new UnsupportedSiteDataFactory(),
new UnsupportedUserGroupFactory(), new UnsupportedHttpClient(),
- executor);
+ executorFactory);
adaptor.init(new MockAdaptorContext(config, null));
ByteArrayOutputStream baos = new ByteArrayOutputStream();
GetContentsRequest request = new GetContentsRequest(
@@ -919,7 +925,7 @@
}
@Test
- public void testGetDocContentListItemWithReadSecurity() throws IOException {
+ public void testGetDocContentListItemWithReadSecurity() throws Exception {
final String getContentListResponse
= loadTestString("sites-SiteCollection-Lists-CustomList-l.xml")
.replace("ReadSecurity=\"1\"", "ReadSecurity=\"2\"");
@@ -1006,13 +1012,7 @@
return new UnsupportedSiteData();
}
},
- mockUserGroupFactory, new UnsupportedHttpClient(),
- new Executor() {
- @Override
- public void execute(Runnable command) {
- command.run();
- }
- });
+ mockUserGroupFactory, new UnsupportedHttpClient(), executorFactory);
final AccumulatingDocIdPusher docIdPusher = new AccumulatingDocIdPusher();
adaptor.init(new MockAdaptorContext(config, null) {
@Override
@@ -1062,7 +1062,7 @@
docIdPusher.getNamedResources());
}
- public void testGetDocContentListItemScopeSameAsParent() throws IOException {
+ public void testGetDocContentListItemScopeSameAsParent() throws Exception {
final String getContentListResponse
= loadTestString("tapasnay-Lists-Announcements-l.xml");
final String getContentListItemResponse
@@ -1114,7 +1114,7 @@
adaptor = new SharePointAdaptor(new UnsupportedSiteDataFactory(),
new UnsupportedUserGroupFactory(), new UnsupportedHttpClient(),
- executor);
+ executorFactory);
adaptor.init(new MockAdaptorContext(config, null));
ByteArrayOutputStream baos = new ByteArrayOutputStream();
GetContentsRequest request = new GetContentsRequest(
@@ -1205,7 +1205,7 @@
}
@Test
- public void testGetDocContentFolder() throws IOException {
+ public void testGetDocContentFolder() throws Exception {
final String getContentListItemResponse
= loadTestString("sites-SiteCollection-Lists-CustomList-1-li.xml");
final String getContentListResponse
@@ -1269,7 +1269,7 @@
adaptor = new SharePointAdaptor(new UnsupportedSiteDataFactory(),
new UnsupportedUserGroupFactory(), new UnsupportedHttpClient(),
- executor);
+ executorFactory);
adaptor.init(new MockAdaptorContext(config, null));
ByteArrayOutputStream baos = new ByteArrayOutputStream();
GetContentsRequest request = new GetContentsRequest(
@@ -1354,10 +1354,10 @@
}
@Test
- public void testGetDocIds() throws IOException, InterruptedException {
+ public void testGetDocIds() throws Exception {
adaptor = new SharePointAdaptor(new UnsupportedSiteDataFactory(),
new UnsupportedUserGroupFactory(), new UnsupportedHttpClient(),
- executor);
+ executorFactory);
AccumulatingDocIdPusher pusher = new AccumulatingDocIdPusher();
adaptor.init(new MockAdaptorContext(config, pusher));
assertEquals(0, pusher.getRecords().size());
@@ -1368,7 +1368,7 @@
}
@Test
- public void testModifiedGetDocIds() throws IOException, InterruptedException {
+ public void testModifiedGetDocIds() throws Exception {
final String getContentVirtualServer
= "<VirtualServer>"
+ "<Metadata URL=\"http://localhost:1/\" />"
@@ -1494,7 +1494,7 @@
"http://localhost:1/_vti_bin/SiteData.asmx");
adaptor = new SharePointAdaptor(siteDataFactory,
new UnsupportedUserGroupFactory(), new UnsupportedHttpClient(),
- executor);
+ executorFactory);
AccumulatingDocIdPusher pusher = new AccumulatingDocIdPusher();
adaptor.init(new MockAdaptorContext(config, pusher));
@@ -1527,8 +1527,7 @@
}
@Test
- public void testModifiedGetDocIdsSP2010() throws IOException,
- InterruptedException {
+ public void testModifiedGetDocIdsSP2010() throws Exception {
final String getContentVirtualServer
= "<VirtualServer>"
+ "<Metadata ID=\"{3a125232-0c27-495f-8c92-65ad85b5a17c}\""
@@ -1608,7 +1607,7 @@
"http://localhost:1/_vti_bin/SiteData.asmx");
adaptor = new SharePointAdaptor(siteDataFactory,
new UnsupportedUserGroupFactory(), new UnsupportedHttpClient(),
- executor);
+ executorFactory);
AccumulatingDocIdPusher pusher = new AccumulatingDocIdPusher();
adaptor.init(new MockAdaptorContext(config, pusher));
@@ -1622,13 +1621,12 @@
}
@Test
- public void testModifiedGetDocIdsClient() throws IOException,
- InterruptedException {
+ public void testModifiedGetDocIdsClient() throws Exception {
final String getChangesContentDatabase
= loadTestString("testModifiedGetDocIdsClient.changes-cd.xml");
adaptor = new SharePointAdaptor(new UnsupportedSiteDataFactory(),
new UnsupportedUserGroupFactory(), new UnsupportedHttpClient(),
- executor);
+ executorFactory);
AccumulatingDocIdPusher pusher = new AccumulatingDocIdPusher();
adaptor.init(new MockAdaptorContext(config, pusher));
SharePointAdaptor.SiteDataClient client = adaptor.new SiteDataClient(
@@ -1652,7 +1650,7 @@
public void testParseError() throws Exception {
adaptor = new SharePointAdaptor(new UnsupportedSiteDataFactory(),
new UnsupportedUserGroupFactory(), new UnsupportedHttpClient(),
- executor);
+ executorFactory);
adaptor.init(new MockAdaptorContext(config, null));
SharePointAdaptor.SiteDataClient client = adaptor.new SiteDataClient(
"http://localhost:1", "http://localhost:1",
@@ -1669,7 +1667,7 @@
config.overrideKey("sharepoint.xmlValidation", "true");
adaptor = new SharePointAdaptor(new UnsupportedSiteDataFactory(),
new UnsupportedUserGroupFactory(),
- new UnsupportedHttpClient(), executor);
+ new UnsupportedHttpClient(), executorFactory);
adaptor.init(new MockAdaptorContext(config, null));
SharePointAdaptor.SiteDataClient client = adaptor.new SiteDataClient(
"http://localhost:1", "http://localhost:1",
@@ -1687,7 +1685,7 @@
public void testDisabledValidation() throws Exception {
adaptor = new SharePointAdaptor(new UnsupportedSiteDataFactory(),
new UnsupportedUserGroupFactory(),
- new UnsupportedHttpClient(), executor);
+ new UnsupportedHttpClient(), executorFactory);
config.overrideKey("sharepoint.xmlValidation", "false");
adaptor.init(new MockAdaptorContext(config, null));
SharePointAdaptor.SiteDataClient client = adaptor.new SiteDataClient(
@@ -1707,7 +1705,7 @@
config.overrideKey("sharepoint.xmlValidation", "true");
adaptor = new SharePointAdaptor(new UnsupportedSiteDataFactory(),
new UnsupportedUserGroupFactory(),
- new UnsupportedHttpClient(), executor);
+ new UnsupportedHttpClient(), executorFactory);
adaptor.init(new MockAdaptorContext(config, null));
SharePointAdaptor.SiteDataClient client = adaptor.new SiteDataClient(
"http://localhost:1", "http://localhost:1",
@@ -2202,11 +2200,4 @@
throw new UnsupportedOperationException();
}
}
-
- private static class UnsupportedExecutor implements Executor {
- @Override
- public void execute(Runnable command) {
- throw new UnsupportedOperationException();
- }
- }
}
