#89 network rendering bug

open-accepted
nobody
None
5
2008-04-21
2008-04-16
Cedric PAILLE
No

Hi,

There's a bug in the rcSend() function in rendererNetwork.cpp, buffer is overwritten by a htonl() function if "toNetwork" parameter is not zero.

I've added temp buffer to avoid this:

void rcSend(SOCKET s,const void *dataToSend,int n,int toNetwork) {
int i,j;
const char *data = NULL;
T32 *buf = NULL;

if (toNetwork) {
buf = (T32*)malloc(n);

if (!buf)
return;

memcpy ( buf, dataToSend, n);

T32* buffer = buf;

for (i=n>>2;i>0;i--,buffer++) {
buffer->integer = htonl(buffer->integer);
}

data = (const char *) buf;
} else{
data = (const char *) dataToSend;
}
j = n;
i = send(s,data,j,0);

if (i <= 0) {
fatal(CODE_SYSTEM,"Connection broken\n");
}

// If we could not send the entire data, send the rest
while(i < j) {
data += i;
j -= i;

i = send(s,data,j,0);

if (i <= 0) {
fatal(CODE_SYSTEM,"Connection broken\n");
break;
}
}

if (buf)
free((void*)data);

stats.totalNetSend += n;
}

Discussion

  • Cedric PAILLE
    Cedric PAILLE
    2008-04-16

    Logged In: YES
    user_id=954786
    Originator: YES

    Oh, forgot to say that it was blocking the deepshadow network-rendering.

    Cheers.

     
  • Logged In: NO

    The original code works well on Unix.

     
  • Okan Arikan
    Okan Arikan
    2008-04-21

    Logged In: YES
    user_id=555300
    Originator: NO

    This is an interesting solution, but I'm hesitant to add it into the trunk because it may use malloc unnecessarily. Most of the code assumes the data that is sent over the network can be modified.

    I isolated the piece of code that does not respect this assumption. This should fix the issue with the deep shadow map rendering problem over the network.

    Cedric, may I ask you to checkout the SVN trunk and see if this problem is still present?

    PS: I'm leaving this bug open until I verify that I fixed it.

    Thank you,

    Okan

     
  • Okan Arikan
    Okan Arikan
    2008-04-21

    • status: open --> open-accepted
     
  • Cedric PAILLE
    Cedric PAILLE
    2008-04-22

    Logged In: YES
    user_id=954786
    Originator: YES

    Hi,

    Yes I've updated the svn.

    The problem I encountered is in this function, look at the variable "sz":

    int CRemoteTSMChannel::sendRemoteBucket(SOCKET s,int x,int y) {
    // record current position, seek back to tile start
    uint64_t curPos = ftell(tsmFile);
    fseek(tsmFile,lastPosition,SEEK_SET);
    uint64_t sz = curPos - lastPosition;
    rcSend(s,&sz,sizeof(uint64_t));

    // send the tile data
    char buf[NETWORK_BUFFER_LENGTH];
    while(sz > 0){
    int nn = (int) ((sz>(NETWORK_BUFFER_LENGTH)) ? (NETWORK_BUFFER_LENGTH) : sz);
    fread(buf,nn,1,tsmFile);
    rcSend(s,buf,nn,FALSE);
    sz -= nn;
    }

    "sz" is byte-swapped (on little-endian machines) and is not valid anymore for the while loop.

    Cheers.