[jcifs] Invalid share causes all free connections consumption

[Michael B Allen]

On Mon, Mar 21, 2011 at 11:06 AM, Gergan Dimitrov
<gergan.dimitrov at gmail.com> wrote:
> Hi Michael,
>
> I tested the proposed fix with 1.3.15 version. I also removed the
> SmbSession.logon() calls. Then the problem for consuming all free
> connections is gone, there is only session for the same host+credentials.
>
> If the logon() is still used though, there are some minor problems - the
> wrongly configured thread produces one more unneeded session, which after
> the time out gets closed by the server. Also, the other correctly configured
> threads for this session seem to be blocked until the time out is over.
>
> I also tested this fix for 1.3.12, but it does not work correctly there -
> after the first attempt, the correct exception about not existing share is
> thrown. But on the next attempt, the exists() method for the SmbFile
> representing the wrong share returns true, which is not correct, of course.
> So if someone plans changing this for lower version - be aware.
>
> So to sum up - with the proposed fix, and not using logon(), everything is
> fine. So Michael, I think you should add this in the todo list, and probably
> mark the logon method as deprecated.

Hi Gergan,

I do not understand. What does this have to do with
SmbSession.logon()? Do you mean the logoff(true) call I referred to in
my previous message?

> I have one more quesiton - when is the next jcfis release planned?

There is nothing currently planned.

Mike

-- 
Michael B Allen
Java Active Directory Integration
http://www.ioplex.com/

> 2011/3/6 Michael B Allen <ioplex at gmail.com>
>>
>> Hi Gergan,
>>
>> I understand now. The logoff(true) destroys the session but there is
>> still a session open on the server. And if you do this in a loop they
>> build up on the Windows side. Yeah, that is not good.
>>
>> I don't recall why logoff(true) is called at this point. Logically it
>> seems it should not be necessary. Just comment those three lines out.
>> And then after two weeks of seeing it work in production, let us know
>> if everything still behaves the same (minus the extra sessions). If it
>> does, I'll modify the code at my end.
>>
>> Mike
>>
>> On Wed, Mar 2, 2011 at 10:23 AM, Gergan Dimitrov
>> <gergan.dimitrov at gmail.com> wrote:
>> > Hi Michael,
>> >
>> > I tested again, and I agree the SmbTransport is reused and no other
>> > sockets
>> > are created. The problem seem to be rather with the SmbSession.
>> >
>> > The logoff() call I meant is in the SmbSession.send() method, line 268
>> > (version 1.3.15). It is called just in the case when the share name is
>> > incorrect. So, when you do treeConnect(), on line 162 a call is made to
>> > session.send(). Then there is a call to the SmbSession.sessionSetup
>> > method.
>> > In this method, line 385, we send some request, which purpose is not
>> > very
>> > clear to me. So the first time, we go through those calls, and since the
>> > share name is wrong, we go here (the SmbSession.send()) :
>> >
>> >  try {
>> >                 transport.send( request, response );
>> >             } catch (SmbException se) {
>> >                 if (request instanceof SmbComTreeConnectAndX) {
>> >                     logoff(true);
>> >                 }
>> >                 request.digest = null;
>> >                 throw se;
>> >             }
>> >
>> >
>> > We go here in the catch, and logoff is called. As a result, the flag
>> > sessionSetup is set to false in the logoff method. On the next time, the
>> > whole cycle repeats, and the check in  the SmbSession.sessionSetup() for
>> > the
>> > flag sessionSetup fails and the whole session setup is done again.
>> >
>> > So I rarther think this continous session setup for the wrong share
>> > causes
>> > the free connections error message, because it is caught just in this
>> > method.
>> >
>> > I also attach a very simple testing program - you have one thread, that
>> > connects to a share on a cycle of 1 second and just calls the exists()
>> > method. You can start also a second thread in parallel, which is for the
>> > same credentials, but for correct host. Actually, I managed to get the
>> > error
>> > with only a Single, wrongly configured thread. To run the application,
>> > just
>> > provide the correct host and user/pass and share name settings in the
>> > main
>> > method.
>> >
>> > So please, take a look at this issue and please give advise - will it be
>> > correct to logoff the whole session in case of wrong share name?
>> >
>> > Regards,
>> > Gergan.
>> >
>> >
>> >
>> > 2011/2/25 Michael B Allen <ioplex at gmail.com>
>> >>
>> >> On Wed, Feb 23, 2011 at 10:10 AM, Gergan Dimitrov
>> >> <gergan.dimitrov at gmail.com> wrote:
>> >> > Hi Michael,
>> >> >
>> >> > unfortunately, I do have other listeners working correctly which
>> >> > connect
>> >> > to
>> >> > the same host, with same credentials, but to correctly defined
>> >> > shares.
>> >> > In
>> >> > this case, I have the problem described above - on each retry of the
>> >> > wrongly
>> >> > configured one, I get one more connection to the server.
>> >> >
>> >> > I did further investigation and I think I found the problem, but I am
>> >> > not
>> >> > sure.
>> >> >
>> >> > The main issue seems to be in the SmbTree.treeConnect method. We have
>> >> > correct host, and correct user/password. This line succeeds:
>> >> >
>> >> > session.transport.connect();
>> >> >
>> >> > At this point, we have a socket open to the remote host.
>> >>
>> >> Hi Gergan,
>> >>
>> >> If you have other resources open with that transport, the same
>> >> transport will be reused and therefore additional sockets should NOT
>> >> be created. If they are, that is a bug which should be investigated
>> >> and fixed.
>> >>
>> >> Note that the code is organized so that:
>> >>
>> >>  SmbFiles share SmbTrees with the same share name
>> >>  SmbTrees share SmbSessions with the same credentials
>> >>  SmbSessions share SmbTransports with the same network address
>> >>
>> >> > We construct then
>> >> > the request and response and call the session.send( request, response
>> >> > );
>> >> > But
>> >> > since the share name is wrong, this fails. As a result, the
>> >> > connectionState
>> >> > is never changed to 2 (so, it is considered not connected), and the
>> >> > variable
>> >> > 'tid' is not initialized. If we see what happens in the send() call,
>> >> > the
>> >> > transport.send( request, response ); throws exception and we go in
>> >> > the
>> >> > catch
>> >> > and call logoff(true).
>> >>
>> >> There is no logoff() call in treeConnect. When the exception is
>> >> caught, it calls treeDisconnect and sets connectionState back to 0. At
>> >> least this is what is supposed to happen.
>> >>
>> >> > Then we go to the the treeDisconnect, the isError is
>> >> > true. But in the treeDisconnect(), we have
>> >> > if (connectionState != 2) // not-connected
>> >> >             return;
>> >> >
>> >> > But here the connectionState is not 2, and we actually do not send a
>> >> > disconnection message to the server.
>> >>
>> >> That is correct. The tree connect never succeeded and therefore no
>> >> disconnect message should be sent.
>> >>
>> >> In CIFS, to get a connection to a file (or share) you must first do a
>> >> "tree connect" which first requires a "session setup" which first
>> >> requires a regular socket. Each socket can have multiple sessions and
>> >> each session can have multiple trees and each tree can have multiple
>> >> files.
>> >>
>> >> > Besides, the isError is also true and
>> >> > tid is 0, and thus none of the conditions to call the disconnect is
>> >> > met.
>> >> > This Tree is practically never disconnected, because since other
>> >> > listeners
>> >> > sharing the same SmbTransport still work, we cannot get read timeout
>> >> > exception and the SmbTransport is never removed. This means the
>> >> > SmbTree
>> >> > is
>> >> > always reused, and always does the frist connect and opens new
>> >> > sockets,
>> >> > that
>> >> > are never closed.
>> >>
>> >> Once the SmbTree object is created, it will remain associated with the
>> >> SmbSession and reused. But when you do a treeConnect and it fails, the
>> >> state should be reset back to 0. Because the existing socket is used,
>> >> there should be no new sockets created and because the connectionState
>> >> should always be reset back to 0 on failure, trying to call
>> >> treeConnect again should go through submitting the
>> >> SmbComTreeConnectAndX message again. At least this is what is supposed
>> >> to happen.
>> >>
>> >> >
>> >> > So Michael, please tell me
>> >> > 1) can you verify this is really the problem. I am still not sure if
>> >> > the
>> >> > upper explains why those connections seem never to close, while there
>> >> > are
>> >> > other running connections to the server.
>> >>
>> >> No. But if you are actually finding that new sockets are being created
>> >> when treeConnect is called on an existing transport that already has
>> >> an open socket, that is an error that should be fixed. But you will
>> >> need to provide evidence that there are multiple sockets being created
>> >> in this scenario.
>> >>
>> >> Create a minimalistic test program that opens a file on a good share
>> >> and then have it try to open a file on a non-existant share 10 times
>> >> (with try / catch inside the loop of course). Then post the code and a
>> >> backtrace to show that multiple threads blocked trying to create
>> >> sockets or whatever it is that is being created that you think JCIFS
>> >> should not be creating.
>> >>
>> >> > 2) what can be the possible solution here. I am not familiar with the
>> >> > purpose of this  SmbComTreeConnectAndX and why it has to be called
>> >> > before we
>> >> > set the state to connected? Also, why not try to disconnect if
>> >> > isError
>> >> > is
>> >> > true on this request? What could be the impact if those checks are
>> >> > removed
>> >> > from the disconnect method?
>> >>
>> >> I think I answered this above.
>> >>
>> >> Mike
>> >>
>> >> --
>> >> Michael B Allen
>> >> Java Active Directory Integration
>> >> http://www.ioplex.com/
>> >>
>> >>
>> >>
>> >> > 2011/2/23 Mohan Radhakrishnan <mohanr at fss.co.in>
>> >> >>
>> >> >> Hi,
>> >> >>
>> >> >>     Does it mean that we should not use SmbSession.logon even using
>> >> >> the
>> >> >> latest JCIFS JAR ?
>> >> >>
>> >> >>
>> >> >> Thanks,
>> >> >> Mohan
>> >> >>
>> >> >> -----Original Message-----
>> >> >> From: jcifs-bounces at lists.samba.org
>> >> >> [mailto:jcifs-bounces at lists.samba.org]
>> >> >> On Behalf Of Michael B Allen
>> >> >> Sent: Wednesday, February 23, 2011 2:35 AM
>> >> >> To: Gergan Dimitrov
>> >> >> Cc: jcifs at lists.samba.org; s.sourkov at bg.seeburger.com
>> >> >> Subject: Re: [jcifs] Invalid share causes all free connections
>> >> >> consumption
>> >> >>
>> >> >> Hi Gergan,
>> >> >>
>> >> >> The SmbSession.logon method is flawed and should no longer be used.
>> >> >> Just do something like (new SmbFile(smb://1.2.3.4/IPC$")).exists().
>> >> >>
>> >> >> But if nothing is listening on the other end it will take a while
>> >> >> for
>> >> >> the socket to timeout. Lowering the jcifs.smb.client.soTimeout might
>> >> >> help with this but you cannot completely eliminate these waiting
>> >> >> threads. It just takes time for the OS to report the SYN packet has
>> >> >> failed.
>> >> >>
>> >> >> Just add logic to slow down the frequency of your attempts. And you
>> >> >> should only use one thread per server. So have one thread try and if
>> >> >> it times out, wait 30 seconds before trying again. If it fails
>> >> >> again,
>> >> >> wait 60 seconds, 120, 240 etc to some maximum like 10 minutes. I
>> >> >> think
>> >> >> a SYN packet failure takes at most about 90 seconds to timeout so as
>> >> >> long as you wait more than that you should not have a net surplus of
>> >> >> transport threads waiting.
>> >> >>
>> >> >> Mike
>> >> >>
>> >> >> On Mon, Feb 21, 2011 at 11:08 AM, Gergan Dimitrov
>> >> >> <gergan.dimitrov at gmail.com> wrote:
>> >> >> > Hi All,
>> >> >> >
>> >> >> > I have an windows share listener, which listens for new files on
>> >> >> > Windows
>> >> >> > share on 1 second interval. I first try to connect to the server
>> >> >> > with
>> >> >> > SmbSession.logon method and then call the SmbFile.exists() method
>> >> >> > for
>> >> >> > the
>> >> >> > directory to listen, both with same user credentials. But if the
>> >> >> > share
>> >> >> > name
>> >> >> > is invalid, after a short period I get run out of free connections
>> >> >> > to
>> >> >> > the
>> >> >> > server. Since the server ip and credentials are correct, the logon
>> >> >> > method
>> >> >> > succeeds. Then the exists() method throws exception. It seems that
>> >> >> > on
>> >> >> > each
>> >> >> > next logon, more connections are created, while the old are not
>> >> >> > closed.
>> >> >> > Of
>> >> >> > course, I could not use the logon method each time, but even if I
>> >> >> > call
>> >> >> > it
>> >> >> > from other application for the same credentials, I get the same
>> >> >> > result.
>> >> >> > If I
>> >> >> > skip the logon part, the problem does not appear, but as I said -
>> >> >> > it
>> >> >> > could
>> >> >> > be called from other application. Please, tell if you have any
>> >> >> > idea
>> >> >> > why
>> >> >> > the
>> >> >> > wrong share causes such behaviour.
>> >> >> >
>> >> >> > The problem can be very easily simulated, but if you need any
>> >> >> > stack
>> >> >> > traces
>> >> >> > or thread dumps, please say so. You can see the growing number of
>> >> >> > IPC$
>> >> >> > connections from the  computer management/ system tools/ shared
>> >> >> > folders/shares tool in Windows. I must add some things:
>> >> >> >
>> >> >> > 1) When the logon succeeds, the SmbTransport created for it is for
>> >> >> > port
>> >> >> > 0,
>> >> >> > while the SmbFile is for port 445, and so at leas two connections
>> >> >> > are
>> >> >> > firstly created. Port was not provided for the logon method, or
>> >> >> > the
>> >> >> > smb
>> >> >> > file
>> >> >> > creation. I tried also with setting port 445 for the logon, but
>> >> >> > the
>> >> >> > result
>> >> >> > was the same
>> >> >> > 2) I could not see more than two SmbTransport objects in the
>> >> >> > CONNECTIONS
>> >> >> > collection.
>> >> >> > 3) when the connection from the SmbFile is successful, but the
>> >> >> > share
>> >> >> > cannot
>> >> >> > be found, the flag treeConnected on the SmbTree objects remains to
>> >> >> > false.
>> >> >> >
>> >> >> > Thank you in advance,
>> >> >> > Gergan.