C++: Type created outside of "try" causes error, but inside, it doesn't

Question

This is - from a pure problem-solving standpoint - is a solved problem but the underlying mechanism might be interesting to some, including me.

I wrote a pretty basic server, listening on a given TCP port. I wrote it in a way, that it can accept multiple requests, not just one and exit, with a while loop.

Since I'm not experienced in C++, I googled a solution and fabricated it to my needs. The part of the code in question looks like this:

    socklen_t addrlen = sizeof(address);

    while(true) {
        try {
            std::cerr << "Listening..." << std::endl;
            if(listen(socketId, 3) < 0) {
                std::cerr << "Error while listening for incoming connections" << std::endl;
                throw;
            }
            processRequest(accept(socketId, (struct sockaddr *) &address, &addrlen));
        } catch(Json::RuntimeError& e) {
            std::cerr << "Json error: " << e.what() << std::endl;
        } catch(...) {
            std::cerr << "Unknown error" << std::endl;
            throw;
        }
    }

The problem with this code is tricky and might be obvious to the more experienced ones of you, but it definitely isn't obvious to me.

I show you a part of the class definition:

    int socketId;
    sockaddr_in address;
    const RequestProcessor& processor;

Notice the order of these variables.

With the method above and the variables in this order, the first request runs smoothly and the software starts listening for the next request as it's intended to.

However, when the second request arrives, the processor reference gets overwritten, presumably because there is some memory overflow - or whatever it's called - happening, which rewrites the memory location the processor reference points to, causing an error somewhere down the line in a method not shown here.

The solution is easy, but can be really hard to find out:

    while(true) {
        try {
            std::cerr << "Listening..." << std::endl;
            if(listen(socketId, 3) < 0) {
                std::cerr << "Error while listening for incoming connections" << std::endl;
                throw;
            }
            socklen_t addrlen = sizeof(address);
            processRequest(accept(socketId, (struct sockaddr *) &address, &addrlen));
        } catch(Json::RuntimeError& e) {
            std::cerr << "Json error: " << e.what() << std::endl;
        } catch(...) {
            std::cerr << "Unknown error" << std::endl;
            throw;
        }
    }

All I did was replace where the addrlen variable is created. This solves the problem and has an added benefit, of being more "reasonable looking".

The question is: why moving the initialization of addrlen solve this problem?

user_11941443 · Answer

accept uses its third argument (i.e. addrlen) as input and output. The input must be the size of the structure that the second argument points to (i.e. address) and the output will be the actual size of the address.

The address may be truncated if it doesn't fit the structure, based on the third argument's input.

If accept returns a larger value for addrlen than sizeof(address), then in the second iteration the argument passed to accept will not match sizeof(address) anymore and undefined behavior will result. In practice accept will write the address beyond address into processor if the class members are layout in that order.

Setting addrlen to sizeof(address) in each loop iteration directly before the call to accept is the correct thing to do and makes sure that this value is always used as input to accept, even if accept modifies its value on output.

Also make sure you are using the appropriate sockaddr_* type for the protocol family you are using for the listening socket, i.e. sockaddr_in for AF_INET (ipv4) or sockaddr_in6 for AF_INET6 (ipv6).

C++: Type created outside of "try" causes error, but inside, it doesn't

1 Answers