Re: [PATCH] wininet: Close sockets open in http requests

[Nigel Liang]

On 10/17/07, Misha Koshelev  wrote:
> On Wed, 2007-10-17 at 17:10 -0500, Misha Koshelev wrote:
> > > n 10/13/07, Nigel Liang <ncliang at gmail.com> wrote:
> > > > Hi,
> > > >
> > > > Http requests open sockets and forget to close them. Left running over a long
> > > > period, the socket file descriptor starts getting large. Once it exceeds
> > > > FD_SETSIZE, the behavior becomes unpredictable and results in funcky crashes.
> > > >
> > > > -Nigel
> > > >
> > > > dlls/wininet/internet.c |    3 +++
> > > >  1 files changed, 3 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/dlls/wininet/internet.c b/dlls/wininet/internet.c
> > > > index 0edca74..8e3a98e 100644
> > > > --- a/dlls/wininet/internet.c
> > > > +++ b/dlls/wininet/internet.c
> > > > @@ -1014,6 +1014,9 @@ BOOL WINAPI InternetCloseHandle(HINTERNE
> > > >          return FALSE;
> > > >      }
> > > > - if (lpwh->htype == WH_HHTTPREQ)
> > > > -
> > > >    NETCON_close(&((LPWININETHTTPREQW)lpwh)->netConnection);
> > > > - WININET_Release( lpwh );
> > > >    WININET_FreeHandle( hInternet );
> > > >
> > > > --
> > > > 1.4.1
> > >
> > > Hi,
> > >
> > > This patch was sent 4 days ago and it hasn't gone in yet. Could anyone
> > > give me some comments on it? Dan filed bug 10032 which is related to
> > > this. While we shouldn't be using select() on high FDs for networking,
> > > this patch closes the sockets that were open by http requests to avoid
> > > getting to high FDs...
> > >
> > > Thanks,
> > > -Nigel
> >
> > Hi, well looking over this very quickly right off the bat it does not seem
> > correct to me. WININET_Release will call the close_connection method of the
> > handle if it is defined, and for WH_HHTTPREQ this is HTTP_CloseConnection,
> > which does call NETCON_Close along with sending all the proper status notifications,
> > etc.
> >
> > So if in fact NETCON_Close is _not_ being called for some reason, you need to track
> > down where involving those functions either why the proper one is not
> > getting called (if that is the case) or if that function somehow needs to call
> > NETCON_Close when it doesn't.
> >
> > Misha
>
> Actually, thinking about this a little more, if this is indeed happening
> (NETCON_close not getting called) then likely this is a reference count
> problem for the object, and thus the proper fix would be to find where
> the reference count is being incremented but not appropriately
> decremented, as info->close_connection only gets called if the reference
> count of the object is zero. In my own experience, there have been quite
> a few places where reference counts were incremented and not decremented
> appropriately in wininet (the ones I ran into I believe are patched now
> though).

Hi,

Yep, you were right. There was a missing call to WININET_Release.
Found it and submitted a new patch:
http://www.winehq.org/pipermail/wine-patches/2007-October/045337.html

Thanks!
-Nigel