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
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).
Misha
On 10/17/07, Misha Koshelev mk144210@bcm.edu 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
On Wed, 2007-10-17 at 17:10 -0700, Nigel Liang wrote:
On 10/17/07, Misha Koshelev mk144210@bcm.edu 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
Glad my hours of staring at wininet led to something useful :) I looked at your new patch and it looks good to me. Now if I could just figure out bug 9922 (or even knew if it still occurs since I can't reproduce it)...
Misha
-
Misha Koshelev -
Nigel Liang