Re: Chromium hangs on Windows XP -> PATCH
Michael Dürig <[email protected]> Thu, 23 Nov 2006 14:33:47 +0100
| Newsgroups | gmane.comp.graphics.chromium.devel |
|---|---|
| Message-ID | <[email protected]> |
This is a multi-part message in MIME format.
--------------060006000200090907070206
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: quoted-printable
X-MIME-Autoconverted: from 8bit to quoted-printable by mr1.bfh.ch id
kANDXm3i031485
Michael D=FCrig wrote:
>>> I use Chromium on Windows XP to drive 6 servers from one client over =
a=20
>>> 1Gbps TCP/IP network. Every now and then Chromium locks up. The lock =
up=20
>>> occurres in __tcpip_read_exact()'s call to recv(). Even though select=
()=20
>>> did report the socket to be ready for reading the first call to recv(=
)=20
>>> blocks. If I kill the server of this connection recv() returns with =
the=20
>>> correct error code. See the stack trace below for more details.
>>> Does anyone else experience similar problems? I suspect this might be=
a=20
>>> bug in some network driver (Intel) but I'm not sure.
>> If there's any bugs in Chromiums packer/unpacker code, a common=20
>> symptom is for the network layer to get stuck in recv() - waiting for=20
>> bytes that aren't coming.
>=20
> I dont think its a bug in Chromium since the block occurs on the first=20
> call to recv() after select() reported the socket to be readable. That=20
> is crTCPIPRecv() calls select(), finds the socket to be readable and=20
> calls crTCPIPReceiveMessage() which blocks right away on the statement
>=20
> if ( __tcpip_read_exact( sock, &len, sizeof(len)) <=3D 0 ).
>=20
> So this really shouldn't block or should it?
>=20
>> Is the lock-up only happening with certain apps? Do those apps work=20
>> ok on other Chromium systems?
>=20
> It does happen with at least a couple of apps I tried, one of which is=20
> atlantis. I didn't test on other systems though.
Ok, some news on this. It does only happen on one machine and on this=20
machine only when running Windows. It does not happen on Linux. Even=20
after changing to a different NIC from a different vendor and a=20
different driver the problem remained. I discussed the issue on=20
microsoft.public.win32.programmer.networks. Someone recommended to use=20
non-blocking sockets instead of blocking sockets because 'select is=20
typically not used with blocking
sockets'. So I hacked tcpip.c such that it now uses non-blocking=20
sockets. I'm not sure if this is of interested to anyone else but just=20
in case, here is the patch.
Michael
--------------060006000200090907070206
Content-Type: text/plain;
name="patch2.patch"
Content-Disposition: inline;
filename="patch2.patch"
Content-Transfer-Encoding: 7bit
Index: util/tcpip.c
===================================================================
RCS file: /cvsroot/chromium/cr/util/tcpip.c,v
retrieving revision 1.92
diff -u -r1.92 tcpip.c
--- util/tcpip.c 26 Oct 2006 23:51:10 -0000 1.92
+++ util/tcpip.c 23 Nov 2006 11:57:35 -0000
@@ -33,6 +33,7 @@
#include <errno.h>
#include <signal.h>
#include <string.h>
+#include <fcntl.h>
#ifdef AIX
#include <strings.h>
#endif
@@ -59,6 +60,8 @@
#ifdef WINDOWS
#define EADDRINUSE WSAEADDRINUSE
#define ECONNREFUSED WSAECONNREFUSED
+#define EWOULDBLOCK WSAEWOULDBLOCK
+#define EINPROGRESS WSAEWOULDBLOCK
#endif
#ifdef WINDOWS
@@ -190,14 +193,42 @@
}
}
+static int
+crSocketReady(CRSocket sock, int mode) {
+ fd_set fds;
+ int r;
+
+ FD_ZERO( &fds );
+ FD_SET( sock, &fds );
+
+ switch(mode) {
+ case 0: // read
+ r = select(sock + 1, &fds, NULL, NULL, NULL);
+ break;
+
+ case 1: // write
+ r = select(sock + 1, NULL, &fds, NULL, NULL);
+ break;
+
+ case 2: // except
+ r = select(sock + 1, NULL, NULL, &fds, NULL);
+ break;
+
+ default:
+ CRASSERT( 0 );
+ return 0;
+ }
+
+ if ( r < 0)
+ crError("crSocketReady( sock=%d ): %s", sock, crTCPIPErrorString( r ));
+
+ return r;
+}
+
cr_tcpip_data cr_tcpip;
-/**
- * Read len bytes from socket, and store in buffer.
- * \return 1 if success, -1 if error, 0 if sender exited.
- */
-int
-__tcpip_read_exact( CRSocket sock, void *buf, unsigned int len )
+static int
+__tcpip_read_exact_intern( CRSocket sock, void *buf, unsigned int len, int block)
{
char *dst = (char *) buf;
/*
@@ -207,37 +238,54 @@
if ( sock <= 0 )
return 1;
- while (len > 0) {
- const int num_read = recv( sock, dst, (int) len, 0 );
- if (num_read < 0) {
- const int error = crTCPIPErrno();
- switch (error) {
- case EINTR:
- crWarning("__tcpip_read_exact() got EINTR, looping");
- continue;
- case EAGAIN:
- continue;
- case EFAULT:
- /* fallthrough */
- case EINVAL:
- /* fallthrough */
- default:
- crWarning("__tcpip_read_exact() error: %s", crTCPIPErrorString(error));
- return -1;
+ while (len > 0) {
+ const int num_read = recv( sock, dst, (int) len, 0 );
+ if (num_read < 0) {
+ const int error = crTCPIPErrno();
+ switch (error) {
+ case EINTR:
+ crWarning("__tcpip_read_exact() got EINTR, looping");
+ continue;
+ case EAGAIN:
+ continue;
+ case EWOULDBLOCK:
+ if (block) {
+ crSocketReady(sock, 0);
+ continue;
+ }
+ else
+ return -EWOULDBLOCK;
+ case EFAULT:
+ /* fallthrough */
+ case EINVAL:
+ /* fallthrough */
+ default:
+ crWarning("__tcpip_read_exact() error: %s", crTCPIPErrorString(error));
+ return -1;
+ }
+ }
+ else if (num_read == 0) {
+ /* client exited gracefully */
+ return 0;
}
- }
- else if (num_read == 0) {
- /* client exited gracefully */
- return 0;
- }
- dst += num_read;
- len -= num_read;
- }
+
+ dst += num_read;
+ len -= num_read;
+ }
return 1;
}
+/**
+ * Read len bytes from socket, and store in buffer.
+ * \return 1 if success, -1 if error, 0 if sender exited.
+ */
+int
+__tcpip_read_exact(CRSocket sock, void *buf, unsigned int len ) {
+ return __tcpip_read_exact_intern(sock, buf, len, 1);
+}
+
void
crTCPIPReadExact( CRConnection *conn, void *buf, unsigned int len )
{
@@ -247,12 +295,8 @@
}
}
-/**
- * Write the given buffer of len bytes on the socket.
- * \return 1 if OK, negative value if error.
- */
-int
-__tcpip_write_exact( CRSocket sock, const void *buf, unsigned int len )
+static int
+__tcpip_write_exact_intern( CRSocket sock, const void *buf, unsigned int len, int block )
{
const char *src = (const char *) buf;
@@ -274,6 +318,10 @@
crWarning("__tcpip_write_exact(TCPIP): caught an EINTR, continuing");
continue;
}
+ else if( err == EWOULDBLOCK && block ) {
+ crSocketReady(sock, 1);
+ continue;
+ }
return -err;
}
@@ -285,6 +333,16 @@
return 1;
}
+/**
+ * Write the given buffer of len bytes on the socket.
+ * \return 1 if OK, negative value if error.
+ */
+int
+__tcpip_write_exact( CRSocket sock, const void *buf, unsigned int len ) {
+ return __tcpip_write_exact_intern(sock, buf, len, 1);
+}
+
+
void
crTCPIPWriteExact( CRConnection *conn, const void *buf, unsigned int len )
{
@@ -349,6 +407,24 @@
crWarning( "setsockopt( TCP_NODELAY=%d )"
" : %s", tcp_nodelay, crTCPIPErrorString( err ) );
}
+
+ {
+#ifdef WINDOWS
+ unsigned long flags = 1;
+ if (ioctlsocket(sock, FIONBIO, &flags) != 0)
+#else
+ int flags;
+ if ( (flags = fcntl(sock, F_GETFL, 0)) == -1 )
+ flags = 0;
+
+ if ( fcntl(sock, F_SETFL, flags | O_NONBLOCK) < 0)
+#endif
+ {
+ int err = crTCPIPErrno( );
+ crError( "Error setting socket to non blocking mode: %s", crTCPIPErrorString( err ) );
+ }
+
+ }
}
@@ -532,11 +608,16 @@
}
addr_length = sizeof( addr );
- conn->tcp_socket = accept( cr_tcpip.server_sock, (struct sockaddr *) &addr, &addr_length );
- if (conn->tcp_socket == -1)
- {
+
+ while ( (conn->tcp_socket = accept( cr_tcpip.server_sock, (struct sockaddr *) &addr, &addr_length )) == -1) {
err = crTCPIPErrno( );
- crError( "Couldn't accept client: %s", crTCPIPErrorString( err ) );
+
+ if (err == EWOULDBLOCK) {
+ crSocketReady(cr_tcpip.server_sock, 0);
+ continue;
+ }
+ else
+ crError( "Couldn't accept client: %s", crTCPIPErrorString( err ) );
}
if (SocketCreateCallback) {
@@ -737,7 +818,6 @@
}
}
-
void
crTCPIPFree( CRConnection *conn, void *buf )
{
@@ -811,53 +891,52 @@
}
}
-
-/**
- * Receive the next message on the given connection.
- * If we're being called by crTCPIPRecv(), we already know there's
- * something to receive.
- */
-static void
-crTCPIPReceiveMessage(CRConnection *conn)
+static int
+crTCPIPReceiveMessage_intern(CRConnection *conn, int block)
{
CRMessage *msg;
CRMessageType cached_type;
CRTCPIPBuffer *tcpip_buffer;
unsigned int len, total, leftover;
const int sock = conn->tcp_socket;
+ int r;
if (conn->type == CR_NO_CONNECTION || !sock) {
/* this might happen during app shut-down */
- return;
+ return 0;
}
- /* Our gigE board is acting odd. If we recv() an amount
- * less than what is already in the RECVBUF, performance
+ /* Our gigE board is acting odd. If we recv() an amount
+ * less than what is already in the RECVBUF, performance
* goes into the toilet (somewhere around a factor of 3).
* This is an ugly hack, but seems to get around whatever
- * funk is being produced
+ * funk is being produced
*
* Remember to set your kernel recv buffers to be bigger
* than the framebuffer 'chunk' you are sending (see
* sysctl -a | grep rmem) , or this will really have no
- * effect. --karl
- */
-#ifdef RECV_BAIL_OUT
+ * effect. --karl
+ */
+#ifdef RECV_BAIL_OUT
{
int inbuf;
(void) recv(sock, &len, sizeof(len), MSG_PEEK);
ioctl(conn->tcp_socket, FIONREAD, &inbuf);
if ((conn->krecv_buf_size > len) && (inbuf < len))
- return;
+ return 0;
}
#endif
/* this reads the length of the message */
- if ( __tcpip_read_exact( sock, &len, sizeof(len)) <= 0 )
- {
+ r = __tcpip_read_exact_intern( sock, &len, sizeof(len), block );
+
+ if ( r == -EWOULDBLOCK)
+ return r;
+
+ else if ( r <= 0) {
__tcpip_dead_connection( conn );
- return;
+ return 0;
}
if (conn->swap)
@@ -900,7 +979,7 @@
total, sock );
crFree( tcpip_buffer );
__tcpip_dead_connection( conn );
- return;
+ return 0;
}
conn->recv_credits -= total;
@@ -927,7 +1006,7 @@
crWarning( "Bad juju: %d %d", tcpip_buffer->allocated, leftover-handled);
crFree( tcpip_buffer );
__tcpip_dead_connection( conn );
- return;
+ return 0;
}
}
@@ -936,9 +1015,6 @@
}
crNetDispatchMessage( cr_tcpip.recv_list, conn, msg, len );
-#if 0
- crLogRead( len );
-#endif
/* CR_MESSAGE_OPCODES is freed in crserverlib/server_stream.c with crNetFree.
* OOB messages are the programmer's problem. -- Humper 12/17/01
@@ -949,8 +1025,19 @@
{
crTCPIPFree( conn, tcpip_buffer + 1 );
}
+
+ return 1;
}
+/**
+ * Receive the next message on the given connection.
+ * If we're being called by crTCPIPRecv(), we already know there's
+ * something to receive.
+ */
+static void
+crTCPIPReceiveMessage(CRConnection *conn) {
+ crTCPIPReceiveMessage_intern(conn, 1);
+}
/**
* Loop over all TCP/IP connections, reading incoming data on those
@@ -964,6 +1051,7 @@
int num_ready, max_fd, i;
fd_set read_fds;
int msock = -1; /* assumed mothership socket */
+ int r;
#ifdef CHROMIUM_THREADSAFE
crLockMutex(&cr_tcpip.recvmutex);
@@ -1056,6 +1144,7 @@
* Loop over connections, receive data on the TCP/IP connections that
* we determined are ready above.
*/
+ r = 0;
for ( i = 0; i < num_conns; i++ )
{
CRConnection *conn = cr_tcpip.conns[i];
@@ -1075,17 +1164,17 @@
if (conn->threaded)
continue;
- crTCPIPReceiveMessage(conn);
+ if (crTCPIPReceiveMessage_intern(conn, 0) > 0)
+ r = 1;
}
#ifdef CHROMIUM_THREADSAFE
crUnlockMutex(&cr_tcpip.recvmutex);
#endif
- return 1;
+ return r;
}
-
static void
crTCPIPHandleNewMessage( CRConnection *conn, CRMessage *msg, unsigned int len )
{
@@ -1298,6 +1387,10 @@
"interruped, trying again", conn->hostname, conn->port );
continue;
}
+ else if ( err == EWOULDBLOCK || err == EINPROGRESS ) {
+ crSocketReady(conn->tcp_socket, 1);
+ return 1;
+ }
else
crWarning( "Couldn't connect to %s:%d, %s",
conn->hostname, conn->port, crTCPIPErrorString( err ) );
--------------060006000200090907070206
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
--------------060006000200090907070206
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
_______________________________________________
Chromium-dev mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/chromium-dev
--------------060006000200090907070206--