OfficeDev / ews-java-api

A java client library to access Exchange web services. The API works against Office 365 Exchange Online as well as on premises Exchange.
MIT License
869 stars 560 forks source link

StreamingSubscription - Error: Connection is still allocated #276

Open avromf opened 9 years ago

avromf commented 9 years ago

For the following code:

        WellKnownFolderName sd = WellKnownFolderName.Inbox;
        FolderId folderId = new FolderId(sd);

        List<FolderId> folder = new ArrayList<FolderId>();
        folder.add(folderId);

        StreamingSubscription subscription = service.subscribeToStreamingNotifications(folder, EventType.NewMail);
        StreamingSubscriptionConnection conn = new StreamingSubscriptionConnection(service, 30);
        conn.addSubscription(subscription);
        EventDelegate delegate = new EventDelegate(service);
        conn.addOnNotificationEvent(delegate);
        conn.addOnDisconnect(delegate);
        conn.open();
        EmailMessage msg = new EmailMessage(service);
        msg.setSubject("Testing Streaming Notification on " + new Date());
        msg.setBody(MessageBody.getMessageBodyFromText("Streaming Notification Test"));
        msg.getToRecipients().add("someone@somewhere.com");
        msg.send();

I get the following error:

 microsoft.exchange.webservices.data.exception.ServiceRequestException: The request failed. Connection is still allocated
 at microsoft.exchange.webservices.data.core.request.SimpleServiceRequestBase.internalExecute(SimpleServiceRequestBase.java:80)
 at microsoft.exchange.webservices.data.core.request.MultiResponseServiceRequest.execute(MultiResponseServiceRequest.java:160)
 at microsoft.exchange.webservices.data.core.ExchangeService.internalCreateItems(ExchangeService.java:592)
 at microsoft.exchange.webservices.data.core.ExchangeService.createItem(ExchangeService.java:651)
 at microsoft.exchange.webservices.data.core.service.item.Item.internalCreate(Item.java:247)
 at microsoft.exchange.webservices.data.core.service.item.EmailMessage.internalSend(EmailMessage.java:147)
 at microsoft.exchange.webservices.data.core.service.item.EmailMessage.send(EmailMessage.java:258)
 at test.TestEWS.main(TestEWS.java:287)
 Caused by: java.lang.IllegalStateException: Connection is still allocated
 at org.apache.http.util.Asserts.check(Asserts.java:34)
 at org.apache.http.impl.conn.BasicHttpClientConnectionManager.getConnection(BasicHttpClientConnectionManager.java:248)
 at org.apache.http.impl.conn.BasicHttpClientConnectionManager$1.get(BasicHttpClientConnectionManager.java:199)
 at org.apache.http.impl.execchain.MainClientExec.execute(MainClientExec.java:173)
 at org.apache.http.impl.execchain.ProtocolExec.execute(ProtocolExec.java:195)
 at org.apache.http.impl.execchain.RetryExec.execute(RetryExec.java:86)
 at org.apache.http.impl.execchain.RedirectExec.execute(RedirectExec.java:108)
 at org.apache.http.impl.client.InternalHttpClient.doExecute(InternalHttpClient.java:184)
 at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:82)
 at microsoft.exchange.webservices.data.core.request.HttpClientWebRequest.executeRequest(HttpClientWebRequest.java:291)
 at microsoft.exchange.webservices.data.core.request.ServiceRequestBase.getEwsHttpWebResponse(ServiceRequestBase.java:780)
 at microsoft.exchange.webservices.data.core.request.ServiceRequestBase.validateAndEmitRequest(ServiceRequestBase.java:719)
 at microsoft.exchange.webservices.data.core.request.SimpleServiceRequestBase.internalExecute(SimpleServiceRequestBase.java:68)
 ... 7 more

I should note that this used to work in the past.

Continuing with some tests on this, creating a separate ExchangeService object for sending the email works. However, the following code still does not work.

        StreamingSubscription subscription = service.subscribeToStreamingNotifications(folder, EventType.NewMail);
        StreamingSubscriptionConnection conn = new StreamingSubscriptionConnection(service, 30);
        conn.addSubscription(subscription);
        conn.open();
        subscription.unsubscribe();
        if (conn.getIsOpen())
            conn.close();

The line subscription.unsubscribe(); will still result in an "Connection is still allocated" error. Also, if I comment it out, the line conn.close(); hangs.

vbauer commented 9 years ago

Btw, maybe it will be better to use PoolingHttpClientConnectionManager instead of BasicHttpClientConnectionManager.

MikeN123 commented 9 years ago

I think the implementation of any streaming requests is quite broken at the moment. We switched to a BasicHttpClientConnectionManager because the old implementation was leaking connections all over the place. I still think this change if fine, but for the streaming subs we need more than 1 connection, so it probably needs it's own connection manager (which would be a pooling one without any limits).

But another issue with the streaming subscriptions is the whole threading model. The current model (see HangingServiceRequestBase) simply spawns it's own thread pool, starts a thread (and then immediately shuts down the thread pool), and will call the callbacks whenever any data is received (and not fully cleanup after itself). I don't think all users are aware of the extra threads being created, which may cause all kinds of multi-threading bugs.

Wouldn't it be better to create some await() of select() call to wait for a response to one or more subscriptions? In that case the developers can choose to handle the multi-threading themselves.

vbauer commented 9 years ago

I've just dug a little bit in the networking part and.. it looks like it should be completely rewritten.

fboucquez commented 7 years ago

+1

What about using protected and factory methods like Spring does so devs can extend clases?

For example, instead of having

HttpClientConnectionManager httpConnectionManager = new BasicHttpClientConnectionManager(registry); in ExchangeServiceBase.initializeHttpClient

you can have a HttpClientConnectionManager httpConnectionManager = createHttpClientConnectionManager(registry); where createHttpClientConnectionManager is a protected method that we can override.

ganeshgowtham commented 7 years ago

I am basically struck at the same issue.

Unable to findItems(...) using the same service instance, I am getting "java.lang.IllegalStateException: Connection is still allocated".

Is there any work around (or) temporary fix for the same ?

avromf commented 7 years ago

@ganeshgowtham Make sure you are using the 2.0 release.

ganeshgowtham commented 7 years ago

I am using version 2.0 as said.

Still facing problem, it works when we have code in sync block.

Connection is leaking and though i have disconnect on service via listener

kamaci commented 7 years ago

I access to e-mails asynchronously but get the same error with version 2.0. I had to do that:

synchronized (this) { message = EmailMessage.bind(service, userItem.getId()); message.load(propertySet); }

However this breaks my async design. Any ideas?

avromf commented 7 years ago

@ganeshgowtham @kamaci Looking at what you both have said, you don't mention anywhere that you were trying to use streaming subscriptions. The original bug that I reported and later fixed was specifically for streaming subscriptions. If you are getting this error and are not using streaming subscriptions, then it maybe it is a separate issue.

ganeshgowtham commented 7 years ago

I am using streaming subscription to get calendar events.

Side note I have another query, for every calendar event creation I am getting atleast 7-8 notification events.

In chaos which one to consume and which one to discard.

-- Thanks Ganesh Gowtham

On 29-May-2017 10:18 PM, "Avrom" notifications@github.com wrote:

@ganeshgowtham https://github.com/ganeshgowtham @kamaci https://github.com/kamaci Looking at what you both have said, you don't mention anywhere that you were trying to use streaming subscriptions. The original bug that I reported and later fixed was specifically for streaming subscriptions. If you are getting this error and are not using streaming subscriptions, then it maybe it is a separate issue.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OfficeDev/ews-java-api/issues/276#issuecomment-304699514, or mute the thread https://github.com/notifications/unsubscribe-auth/ADTVb18V3NJcvgOo2XxD8BzQFTyvR7O6ks5r-vbCgaJpZM4D4k18 .

avromf commented 7 years ago

@ganeshgowtham Do you have some sample code for testing this?

kamaci commented 7 years ago

@avromf I do not use streaming subscription. Is it a known bug that I should synchronize access to ExchangeService object?

kamaci commented 7 years ago

@avromf OK I see that it is not thread safe: https://msdn.microsoft.com/en-us/library/microsoft.exchange.webservices.data.exchangeservice(v=exchg.80).aspx

alexmreis commented 6 years ago

Issue is still there in 2018. Happy 3 year anniversary (almost)!

ganeshgowtham commented 6 years ago

Issue is fixed, this might be because of synchronization.

On Jan 26, 2018 11:25 PM, "Alexandre M. Reis" notifications@github.com wrote:

Issue is still there in 2018. Happy 3 year anniversary (almost)!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OfficeDev/ews-java-api/issues/276#issuecomment-360856710, or mute the thread https://github.com/notifications/unsubscribe-auth/ADTVbwU6_jJSI6pzX9GSE4AwJVB6CLekks5tOhGPgaJpZM4D4k18 .