1

I am writing a bittorrent client and need to contact multiple trackers over a tcp connection. In order to do this I wrote a winsock wrapper class shown below:

class trackerSocket{
public:
    ~trackerSocket();

    int trackerInitialize(string address);
    int trackerSend(string getParams);
    int trackerRecv();

    be_node *responseDict;
    bool working;

private:
    string address;
    string port;
    string protocol;
    string page;
    SOCKET ConnectSocket;

    int parseAnnounce(string announce);
    int parseTrackerResponse(string response);
};

The program starts by assigning a new trackerSocket class to a variable. The trackerInitialize function is called on this function and if successful, the class is pushed onto a vector to store all the working trackers. Here is the trackerInitialize function:

int trackerSocket::trackerInitialize(string announce){
    WSADATA wsaData;
    int iResult;

    working = true;

    iResult = parseAnnounce(announce);
    if(iResult != 0){
        working = false;
        return iResult;
    }

    //Initialize Winsock
    iResult = WSAStartup(MAKEWORD(2,2), &wsaData);
    if(iResult != 0){
        return 1;
    }

    struct addrinfo *result = NULL,
                    *ptr  = NULL,
                    hints;

    ZeroMemory(&hints, sizeof(hints));
    hints.ai_family = AF_INET;
    hints.ai_socktype = SOCK_STREAM;
    hints.ai_protocol = IPPROTO_TCP;

    //Resolve the server address and port
    iResult = getaddrinfo(address.c_str(), port.c_str(), &hints, &result);
    if(iResult != 0){
        WSACleanup();
        return 1;
    }

        ConnectSocket = INVALID_SOCKET;

    //Attempt to connect to the first address returned by
    //the call to getaddrinfo
    ptr = result;

    do{
        //Create a socket for connecting to the server
        ConnectSocket = socket(ptr->ai_family, ptr->ai_socktype, ptr->ai_protocol);
        if(ConnectSocket == INVALID_SOCKET){
            ptr = ptr->ai_next;
            continue;
        }

        //Connect to server
        iResult = connect(ConnectSocket, ptr->ai_addr, (int)ptr->ai_addrlen);
        if(iResult != SOCKET_ERROR){
            break;
        } else {
            closesocket(ConnectSocket);
            ConnectSocket = INVALID_SOCKET;
            ptr = ptr->ai_next;
        }
    } while(ptr != NULL);

    freeaddrinfo(result);
    if(ConnectSocket == INVALID_SOCKET){
        working = false;
        WSACleanup();
        return 1;
    }

    return 0;
}

Then the program performs code to generate the message to be sent to the trackers. For each tracker class in the vector it's trackerSend function is called with the message, here is the trackerSend function:

int trackerSocket::trackerSend(string getParams){
    int iResult;

    ostringstream os;
    os << "GET " << page << getParams << " HTTP/1.1\r\n"
        << "Host: " << address << "\r\n"
        << "Accept: text/html\r\n"
        << "\r\n";

    string sendBuf = os.str();

    //Send tracker request
    iResult = send(ConnectSocket, sendBuf.c_str(), strlen(sendBuf.c_str()), 0);
    if(iResult == SOCKET_ERROR){
        working = false;
        closesocket(ConnectSocket);
        WSACleanup();
        return 1;
    }

    return 0;
}

Every time the program runs the send function returns -1 for every tracker. If I call the WSAGetLastError() function it returns 10093. The msdn definition for this error is:

Successful WSAStartup not yet performed. Either the application has not called WSAStartup or WSAStartup failed. The application may be accessing a socket that the current active task does not own (that is, trying to share a socket between tasks), or WSACleanup has been called too many times.

I can't see that WSACleanup has been called too many times so I can only assume that the socket is not owned by the current active task (no idea what that means), can anyone see the problem?

Heres some of the code for the main program (which I have described above):

//Store tracker URL's in vector
vector<trackerSocket> trackers;
trackerSocket *temptracker = new trackerSocket();
iResult = temptracker->trackerInitialize(announce);
if(iResult == 0){
    trackers.push_back(*temptracker);
}
if(announcelist != NULL){
    i = 0;
    while(announcelist[i]){
        if(strcmp(announcelist[i]->val.l[0]->val.s, announce.c_str()) != 0){
            temptracker = new trackerSocket();
            iResult = temptracker->trackerInitialize(announcelist[i]->val.l[0]->val.s);
            if(iResult == 0){
                trackers.push_back(*temptracker);
            }
        }
        i++;
    }
}

//Check that at least one of the tracker URL's was valid
if(trackers.size() == 0){
    printf("None of the tracker URL's provided were valid.\n");
    return 1;
}

//Generate some required values
string peerid = genPeerID();
string peerport = "12345";
int uploaded = 0;
int downloaded = 0;

//Work out how many bytes are left to download
int left = 0;
if(singlefile){
    left = length;
} else {
    for(i = 0; i < filesinfo.size(); i++){
        left += filesinfo[i].length;
    }
}

//Send GET Request to tracker
i = 0;
ostringstream os;
string getParams;
string response;
os << "info_hash=" << infohash << "&peer_id=" << peerid << "&port=" << peerport <<
    "&uploaded=" << uploaded << "&downloaded=" << downloaded << "&event=started";

getParams = os.str();
do{     
    iResult = trackers[i].trackerSend(getParams);
    if(iResult != 0){
        printf("trackerSend %d failed: %d\n", i, iResult);
        i++;
        continue;
    }
} while(i < trackers.size());
2
  • Is there any way you can condense your question to be about 1/4 of it's current length? The way it is right now it seems to have too much information (I got tired reading through it...)
    – YePhIcK
    Jul 25, 2012 at 12:06
  • I thought someone might say that :( Unfortunately I'm not sure where the bug in the program lies so I have to include all the code I think might contain it, I did my best to include what I thought was the most relevant code.
    – brnby
    Jul 25, 2012 at 12:30

2 Answers 2

2

From your code you shouldn't call WSACleanup on the send, if one tracker fails to send you will decrease the internal counter and if it reaches 0 the system will require a new WSAStartup that you aren't calling...

I suggest you call only one time the WSAStartup (ex. at the start of the application) and only one time the WSACleanup. (ex. end of application)

5
  • However, the trackerInitialize function calls WSAStartup every time it is called. It is called once for each tracker I believe and WSACleanup is called if a tracker fails. I don't think that WSACleanup is being called more times than WSAStartup but I may be wrong
    – brnby
    Jul 25, 2012 at 13:34
  • can you post the while condition in the do { ... } ? Jul 25, 2012 at 13:45
  • The code calls WSACleanup too many times. It class WSAStartup once, but calls WSACleanup once per connection. Jul 25, 2012 at 13:48
  • Added the while condition after the do. Am I wrong in thinking that temptracker->trackerInitialize is called once for each tracker in the while(announcelist[i]) loop in the last block of code? And the trackerInitialize function calls WSAStartup?
    – brnby
    Jul 25, 2012 at 16:35
  • 1
    The tracker needs to keep track of whether it calls WSAStartup() (and how many times) and call WSACleanup() for each successful WSAStartup() call, rather than calling it unconditionally. If the tracker does not call WSAStartup(), or if WSAStartup() fails, calling WSACleanup() the wrong number of times will unbalance WinSock's reference count. Jul 25, 2012 at 20:11
1

I have managed to figure out the problem after taking a break from the code. I believe that the problem is described in this SO question and answer:

C++ vector of objects vs. vector of pointers to objects

What you have to know about vectors in c++ is that they have to use the copy operator of the class of your objects to be able to enter them into the vector. If you had memory allocation in these objects that was automatically desallocated when the destructor was called, that could explain your problems : your object was copied into the vector then destroyed.

If you have, in your object class, a pointer that points towards a buffer allocated, a copy of this object will point towards the same buffer (if you use the default copy operator). If the destructor desallocates the buffer, when the copy destructor will be called, the original buffer will be desallocated, therefore your data won't be available anymore.

This problem doesn't happen if you use pointers, because you control the life of your elements via new/destroy, and the vector functions only copy pointer towards your elements.

After changing the vector to store pointers to the trackerSocket class instead of just copies of trackerSocket classes and changing the calls to the trackerSocket functions in the vector (. to ->) the problem was successfully fixed. Here are the changes I made:

Old:

vector<trackerSocket> trackers;
trackers.push_back(*temptracker);
trackers[i].trackerSend(getParams);

New:

vector<trackerSocket*> trackers;
trackers.push_back(temptracker);
trackers[i]->trackerSend(getParams);

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.