[Tickets #12837] Re: Imap_Client: Leaking IMAP socket on error

Discussion:

(too old to reply)

n***@bugs.horde.org

2013-11-14 20:21:59 UTC

When an IMAP auth error is thrown, the socket to the IMAP server is
not closed.

That is correct. An authorization error shouldn't close the
connection. It just indicates that the transition between
non-authenticated -> authenticated state was not successful. But at
the protocol level you are not disconnected from the IMAP server and,
in fact, the IMAP server remains willing/able to process further
commands.

For example, in an interactive environment - such as a CLI script -
there is no need to destroy the IMAP connection. If, after an
unsuccessful authentication attempt, the CLI can ask for another
password and we should be reusing the same connection for
authentication the next time we try to authenticate.

I've created the attached example script to demonstrate the problem.

I'm confused by this script. First, it's going to continually loop
whether or not authentication is correct or not. So I'm not sure what
the outer while() loop is attempting to show.

Second, you are not distinguishing between what the error is from
listMailboxes(). listMailboxes() may fail because of an
authentication error. But it also may fail because of invalid input.
Or because of a temporary error on the IMAP server. Or because you
don't have the correct ACL permissions. Simply checking for an
Exception in this case is insufficient.

It sounds like this is more of a logic error in the Kolab code. For
starters, the Horde_Imap_Client instantiation code should only occur
once and should be disconnected from any of the actions on the object
itself. And if an IMAP action throws an exception, you surely need to
break any loop containing that IMAP action. But this doesn't have
anything to do with authentication errors per se. That just seems to
be deficient exception handling in general.

This could be a failure of the documentation to explain the different
types of Exceptions. But it doesn't seem to be a design error in
Imap_Client.

Could we close the socket on error?

You can do it manually by calling logout(). But see above: there
shouldn't be any reason to. See, e.g., IMP: we never call logout().

--
bugs mailing list
Frequently Asked Questions: http://wiki.horde.org/FAQ
To unsubscribe, mail: bugs-***@lists.horde.org

n***@bugs.horde.org

2013-11-15 10:43:12 UTC

Permalink

DO NOT REPLY TO THIS MESSAGE. THIS EMAIL ADDRESS IS NOT MONITORED.

Ticket URL: http://bugs.horde.org/ticket/12837
------------------------------------------------------------------------------
Ticket | 12837
Updated By | Thomas Jarosch <***@intra2net.com>
Summary | Imap_Client: Leaking IMAP socket on error
Queue | Horde Framework Packages
Version | Git master
Type | Bug
State | Feedback
Priority | 1. Low
Milestone |
Patch |
Owners |
------------------------------------------------------------------------------

Thomas Jarosch <***@intra2net.com> (2013-11-15 10:43) wrote:

I've just quickly hacked together the test script to check if it
really allocates a socket to the IMAP server when an exception is
thrown. listMailboxes() was added to do something on the socket.
(I had to "reverse diagnose" the issue since I just saw the server ran
out of sockets.)

It seems to me the PHP gc does not clean up the $imap object when it's
still inside the loop.

I'm not sure how I can see what part of horde actually caused the
problem on a productive box, ActiveSync was not in use and still
something got stuck in an endless loop. We have an unlimited
maximum_execution_time and I now changed it to 43200 seconds. That
should be way enough.

I'll check the Kolab code if it properly reuses the Imap_Client
object, that's a good hint.

--
bugs mailing list
Frequently Asked Questions: http://wiki.horde.org/FAQ
To unsubscribe, mail: bugs-***@lists.horde.org

n***@bugs.horde.org

2013-11-18 22:50:15 UTC

Permalink

Post by n***@bugs.horde.org
I've just quickly hacked together the test script to check if it
really allocates a socket to the IMAP server when an exception is
thrown. listMailboxes() was added to do something on the socket.
(I had to "reverse diagnose" the issue since I just saw the server
ran out of sockets.)
It seems to me the PHP gc does not clean up the $imap object when
it's still inside the loop.

That's correct. Why would it? The $imap object is successfully
created. It's not until the listMailboxes call is run that the
exception is thrown. But all code/variables before that in the try
block isn't backed-out. Thus, $imap still exists after the
listMailboxes call fails.

If you want to explicitly check whether the $imap object can actually
connect to the server, you need to manually call $imap->login().

Post by n***@bugs.horde.org
I'm not sure how I can see what part of horde actually caused the
problem on a productive box, ActiveSync was not in use and still
something got stuck in an endless loop. We have an unlimited
maximum_execution_time and I now changed it to 43200 seconds. That
should be way enough.

That isn't desirable. If something is not finishing in 30 seconds,
then there is something wrong with the script. Bumping the max
execution time isn't going to help.

Post by n***@bugs.horde.org
I'll check the Kolab code if it properly reuses the Imap_Client
object, that's a good hint.

My guess is that this is the issue. Or else, an Exception is not
being handled correctly somewhere.

Within a self-contained piece of code - say the Kolab library - you
only need a single IMAP object that can be shared between all code.

--
bugs mailing list
Frequently Asked Questions: http://wiki.horde.org/FAQ
To unsubscribe, mail: bugs-***@lists.horde.org

n***@bugs.horde.org

2014-01-08 18:45:09 UTC

Permalink

--
bugs mailing list
Frequently Asked Questions: http://wiki.horde.org/FAQ
To unsubscribe, mail: bugs-***@lists.horde.org

n***@bugs.horde.org

2014-01-13 08:45:07 UTC

Permalink

Post by n***@bugs.horde.org

That's correct. Why would it? The $imap object is successfully
created. It's not until the listMailboxes call is run that the
exception is thrown. But all code/variables before that in the try
block isn't backed-out. Thus, $imap still exists after the
listMailboxes call fails.
If you want to explicitly check whether the $imap object can
actually connect to the server, you need to manually call
$imap->login().

oh well, I thought the exception would be handled in the same
way a C++ exception would be handled. In that case the $imap
object would have been destroyed upon leaving the try/catch block.
Guess that was a wrong assumption ;)

Post by n***@bugs.horde.org

That isn't desirable. If something is not finishing in 30 seconds,
then there is something wrong with the script. Bumping the max
execution time isn't going to help.

doing large imports of 5.000+ events via CSV sometimes takes a few minutes
and I certainly ran into timeouts. The Kolab backend is a lot slower than
the SQL backend during first imports. I had to increase the max_input_time
and max_execution_time to work around this.

Post by n***@bugs.horde.org

Post by n***@bugs.horde.org
I'll check the Kolab code if it properly reuses the Imap_Client
object, that's a good hint.

My guess is that this is the issue. Or else, an Exception is not
being handled correctly somewhere.
Within a self-contained piece of code - say the Kolab library - you
only need a single IMAP object that can be shared between all code.

yep, exactly. I've seen you did some changes to the code
to improve the situation. I've not tested them yet, I'm still
in the process of upgrading to cyrus 2.4 (and thus having a CONDSTORE server).

Feel free to close this issue. Thanks!
Thomas

--
bugs mailing list
Frequently Asked Questions: http://wiki.horde.org/FAQ
To unsubscribe, mail: bugs-***@lists.horde.org