List Info

Thread: CR: large leak in the server (CHXSocket, CHTTPDemux, etc.)




CR: large leak in the server (CHXSocket, CHTTPDemux, etc.)
user name
2006-06-05 20:21:31
Synopsis
========
Fixes large leak in the server (CHXSocket, CHTTPDemux,
CRN1CloakGETHandler, RTSPProtocol, etc.)

Branches: HEAD
Suggested Reviewer: anyone


Description
===========
Occasionally during an uptime or other load test, we would
have a
CHXSocket::OnEvent happen, with the even type read. For http
(and
probably others), the socket would call EventPending on the
response.
The response (CHTTPDemux in this case) would try to read,
but in the
mean time something happened to the client, and we would
have a
CONNRESET error.

The protocol calls Close(FAIL) on itself, which calls
HXProtocol:one(FAIL
), which calls CHXServSocket::Close,
CHXSocket::Close. The problem is that if we try to close the
socket, but
the write Q isn't empty, then the socket tries to
'linger' to finish the
write. This never happens, and we are left with a leaked
socket,
response, and all the stuff underneath.

The fix is to detect a read error in CHXSocket:oRead,
and if it fails
then empty the write queue before returning an error. That
way we won't
get in a lingering state, and the behavior will remain the
same otherwise.


Files Affected
==============
common/netio/platform/posix/sockimp.cpp


Testing Performed
=================
Unit Tests:
n/a

Integration Tests:
- Verified that socket no longer gets in lingering state
after a
CONNRESET read error.

Leak Tests:
- New uptime style lct will confirm fix.

Performance Tests:
n/a

Platforms Tested: linux-rhel4-i686
Build verified: linux-rhel4-i686, win32-i386-vc7


QA Hints
========
n/a

Index: platform/posix/sockimp.cpp
============================================================
=======
RCS file: /cvsroot/common/netio/platform/posix/sockimp.cpp,v
retrieving revision 1.95
diff -u -w -r1.95 sockimp.cpp
--- platform/posix/sockimp.cpp	20 Apr 2006 23:48:01
-0000	1.95
+++ platform/posix/sockimp.cpp	3 Jun 2006 01:14:23 -0000
 -1782,6
+1782,12 
             {
                 HX_RELEASE(*ppAddr);
             }
+            // if we don't flush the write queue on a
socket error then
+            // we wind up stuck in a lingering state after
Close()
+            if (hxr == HXR_SOCK_CONNRESET)
+            {
+                m_pWriteQueue->Discard();
+            }
         }
     }
     return hxr;

_______________________________________________
Common-dev mailing list
Common-devhelixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/comm
on-dev
Resend: CR: large leak in the server (CHXSocket, CHTTPDemux, etc.)
user name
2006-06-07 17:02:16
Could someone please take a look at this? I'd like to get
it checked in 
to the head today and I need approval from someone on the
common-dev list.

Thanks,

Sean

Synopsis
========
Fixes large leak in the server (CHXSocket, CHTTPDemux,
CRN1CloakGETHandler, RTSPProtocol, etc.)

Branches: HEAD
Suggested Reviewer: anyone


Description
===========
Occasionally during an uptime or other load test, we would
have a
CHXSocket::OnEvent happen, with the even type read. For http
(and
probably others), the socket would call EventPending on the
response.
The response (CHTTPDemux in this case) would try to read,
but in the
mean time something happened to the client, and we would
have a
CONNRESET error.

The protocol calls Close(FAIL) on itself, which calls
HXProtocol:one(FAIL
), which calls CHXServSocket::Close,
CHXSocket::Close. The problem is that if we try to close the
socket, but
the write Q isn't empty, then the socket tries to
'linger' to finish the
write. This never happens, and we are left with a leaked
socket,
response, and all the stuff underneath.

The fix is to detect a read error in CHXSocket:oRead,
and if it fails
then empty the write queue before returning an error. That
way we won't
get in a lingering state, and the behavior will remain the
same otherwise.


Files Affected
==============
common/netio/platform/posix/sockimp.cpp


Testing Performed
=================
Unit Tests:
n/a

Integration Tests:
- Verified that socket no longer gets in lingering state
after a
CONNRESET read error.

Leak Tests:
- New uptime style lct will confirm fix.

Performance Tests:
n/a

Platforms Tested: linux-rhel4-i686
Build verified: linux-rhel4-i686, win32-i386-vc7


QA Hints
========
n/a



Index: platform/posix/sockimp.cpp
============================================================
=======
RCS file: /cvsroot/common/netio/platform/posix/sockimp.cpp,v
retrieving revision 1.95
diff -u -w -r1.95 sockimp.cpp
--- platform/posix/sockimp.cpp	20 Apr 2006 23:48:01
-0000	1.95
+++ platform/posix/sockimp.cpp	3 Jun 2006 01:14:23 -0000
 -1782,6
+1782,12 
              {
                  HX_RELEASE(*ppAddr);
              }
+            // if we don't flush the write queue on a
socket error then
+            // we wind up stuck in a lingering state after
Close()
+            if (hxr == HXR_SOCK_CONNRESET)
+            {
+                m_pWriteQueue->Discard();
+            }
          }
      }
      return hxr;


_______________________________________________
Common-dev mailing list
Common-devhelixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/comm
on-dev
Resend: CR: large leak in the server (CHXSocket, CHTTPDemux, etc.)
user name
2006-06-08 16:28:55
This looks good to me.

Eric 

> -----Original Message-----
> From: common-dev-bounceshelixcommunity.org 
> [mailto:common-dev-bounceshelixcommunity.org] On
Behalf Of Sean Smith
> Sent: Wednesday, June 07, 2006 1:02 PM
> To: common-devhelixcommunity.org
> Subject: [Common-dev] Resend: CR: large leak in the
server 
> (CHXSocket,CHTTPDemux, etc.)
> 
> Could someone please take a look at this? I'd like to
get it 
> checked in 
> to the head today and I need approval from someone on
the 
> common-dev list.
> 
> Thanks,
> 
> Sean
> 
> Synopsis
> ========
> Fixes large leak in the server (CHXSocket, CHTTPDemux,
> CRN1CloakGETHandler, RTSPProtocol, etc.)
> 
> Branches: HEAD
> Suggested Reviewer: anyone
> 
> 
> Description
> ===========
> Occasionally during an uptime or other load test, we
would have a
> CHXSocket::OnEvent happen, with the even type read. For
http (and
> probably others), the socket would call EventPending on
the response.
> The response (CHTTPDemux in this case) would try to
read, but in the
> mean time something happened to the client, and we
would have a
> CONNRESET error.
> 
> The protocol calls Close(FAIL) on itself, which calls
> HXProtocol:one(FAIL
), which calls CHXServSocket::Close,
> CHXSocket::Close. The problem is that if we try to
close the 
> socket, but
> the write Q isn't empty, then the socket tries to
'linger' to 
> finish the
> write. This never happens, and we are left with a
leaked socket,
> response, and all the stuff underneath.
> 
> The fix is to detect a read error in CHXSocket:oRead,
and 
> if it fails
> then empty the write queue before returning an error.
That 
> way we won't
> get in a lingering state, and the behavior will remain
the 
> same otherwise.
> 
> 
> Files Affected
> ==============
> common/netio/platform/posix/sockimp.cpp
> 
> 
> Testing Performed
> =================
> Unit Tests:
> n/a
> 
> Integration Tests:
> - Verified that socket no longer gets in lingering
state after a
> CONNRESET read error.
> 
> Leak Tests:
> - New uptime style lct will confirm fix.
> 
> Performance Tests:
> n/a
> 
> Platforms Tested: linux-rhel4-i686
> Build verified: linux-rhel4-i686, win32-i386-vc7
> 
> 
> QA Hints
> ========
> n/a
> 
> 
> 
> Index: platform/posix/sockimp.cpp
>
============================================================
=======
> RCS file:
/cvsroot/common/netio/platform/posix/sockimp.cpp,v
> retrieving revision 1.95
> diff -u -w -r1.95 sockimp.cpp
> --- platform/posix/sockimp.cpp	20 Apr 2006 23:48:01 
> -0000	1.95
> +++ platform/posix/sockimp.cpp	3 Jun 2006 01:14:23
-0000
>  -1782,6 +1782,12 
>               {
>                   HX_RELEASE(*ppAddr);
>               }
> +            // if we don't flush the write queue on a
socket 
> error then
> +            // we wind up stuck in a lingering state
after Close()
> +            if (hxr == HXR_SOCK_CONNRESET)
> +            {
> +                m_pWriteQueue->Discard();
> +            }
>           }
>       }
>       return hxr;
> 
> 
> _______________________________________________
> Common-dev mailing list
> Common-devhelixcommunity.org
> http://lists.helixcommunity.org/mailman/listinfo/comm
on-dev
> 


_______________________________________________
Common-dev mailing list
Common-devhelixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/comm
on-dev
[1-3]

about | contact  Other archives ( Real Estate discussion Medical topics )