Avatar of summer_soccer
summer_soccer
 asked on

pthreads to resolve ip's dns name? What's wrong?

Hi there,

I wrote a program, but the output file contains incorrect resolved dns names. Can anybody take a look, and point out what's wrong with my program? Thanks.

#include <stdio.h>      // for fprintf()
#include <netdb.h>      // for gethostbyname()
#include <stdlib.h>     // for exit()
#include <unistd.h>     // for fork()
#include <pthread.h>             // for mutex and threads operations

#define MAXIPS  500000
#define MAXTHREADS 150

//struct in_addr  *ipaddr;
char addrstr[256];
char *addrptr[MAXIPS];
char *hostptr[MAXIPS];
int totalips;
int len;
int i;


int totalresolvedips = 0;
pthread_mutex_t lock;


static void *resolver(void *data) {
    int index;
    struct hostent *host;
    struct in_addr *ipaddr;

    while(1) {

        pthread_mutex_lock(&lock);
        if(totalresolvedips < totalips) {
            index = totalresolvedips;
            totalresolvedips++;
            pthread_mutex_unlock(&lock);
            hostptr[index] = (char *)malloc(256);

           // oo a gethostbyaddr() to get a pointer to struct host

           ipaddr=(struct in_addr*)malloc(sizeof(struct in_addr));
           ipaddr->s_addr=inet_addr(addrptr[index]) ;

           host = gethostbyaddr(ipaddr, sizeof(in_addr_t), AF_INET);
           
           printf("Resolved %d out of %d\n", index, totalips);
   
           // output host name if host found
           if(host == NULL) {
               hostptr[index] = "host_no_name";
           }
           else {
               strcpy(hostptr[index], host->h_name);
           }
        }
        else {  // all ips have been resolved
            pthread_mutex_unlock(&lock);

            break;
        }
    }

    return NULL;

}

int main(int argc, char *argv[]) {
   
    FILE *ifp, *ofp;
    pthread_t ths[MAXTHREADS];
    void *retval;

    pthread_mutex_init(&lock, NULL);

    if(argc != 3) {    /* Test for correct number of arguments */
        fprintf(stderr, "Usage: %s <ipfile-to-be-resolved> <ipdnsfile-output>\n", argv[0]);
        exit(1);
    }

    if((ifp = fopen(argv[1], "r")) == NULL) {
        fprintf(stderr, "%s: can't open %s\n", argv[0], argv[1]);
        exit(1);
    }

    if((ofp = fopen(argv[2], "w")) == NULL) {
        fprintf(stderr, "%s: can't open %s\n", argv[0], argv[2]);
        exit(1);
    }

    totalips = 0;

    while(fgets(addrstr, 256, ifp) != NULL) {
        // printf("%s", addrstr);
       
        // remove newline
        len = strlen(addrstr);
       
        if(addrstr[len-1] == '\n')
            addrstr[len-1] = 0;

        addrptr[totalips] = (char *)malloc(256);
        strcpy(addrptr[totalips], addrstr);

        totalips++;
    }

    for(i=0; i<MAXTHREADS; i++) {
        pthread_create(&ths[i], NULL, resolver, 0);
    }

    for(i=0; i<MAXTHREADS; i++) {
        pthread_join(ths[i], &retval);
    }

    for(i=0; i<totalips; i++) {
        fprintf(ofp, "%s\t\t%s\n", addrptr[i], hostptr[i]);
    }

    fclose(ifp);
    fclose(ofp);
C

Avatar of undefined
Last Comment
summer_soccer

8/22/2022 - Mon
avsrivastava

Could you post a sample line from the input text file, the corresponding output line and the expected correct output.
See if your /etc/hosts.conf and /etc/resolv.conf files are ok.

One thing I can tell you for sure is that you are only wasting resources by using pthreads. Unless this is an assignment, the way threads have been used will not improve any performance, only decrease it. This is why I say this:

The critical section has been (correctly) protected by the mutex so at max one thread will be in the section. There is absolutely no parallelism in the program, so all but one created threads wait for the mutex and only one thread would do all the job.
(The first thread to get hold of the mutex will hold it till all job is done, and when it releases the mutex for the first time, all threads will find the if condition in the resolver function false and will simply exit without doing anything useful at all.)
Removing the mutex would not help because that part of code is not thread safe.

So, I would suggest you to remove the threading all together, and then try running it in the debugger.
summer_soccer

ASKER
While only variable "totalresolvedips" is in the cirtical section and is protected by the mutex lock, the most time-consuming function gethostbyaddr( ) should be able to parallelize, because all threads can execute that function in parallel, right? When one thread is waiting for the resolving results, it can go to sleep, and the cpu can schedule another thread to execute gethostbyaddr( ), right?

Below are three sample output lines. For anonymity, I changed the resolved dns names, but all three ip addresses have the same incorrect dns names.

69.88.128.2             somehost.somedepartment.someuniversity.edu
221.135.216.1           somehost.somedepartment.someuniversity.edu
217.80.0.1              somehost.somedepartment.someuniversity.edu
avsrivastava

oh my bad.
I did not see the lock was released inside the if block, but now I know the problem.

The problem is that the gethostbyaddr function is not thread safe. So, while going through the code last time I just had a feeling that you knew that and would protect it with the mutex. Here is a snip from the man page of gethostbyaddr.

<snip>
NOTES
 The functions gethostbyname() and gethostbyaddr() may return pointers to static data, which may be overwritten by later calls. Copying the struct hostent does not suffice, since it contains pointers - a deep copy is required.
</snip>

So you have to ensure that the 'host' struct gets copied before someother other thread calls the function. But since that part is not protected, the datastructure gets overwritten(which explains the same values of each).

To test if this is the problem just comment out the mutex release in the if block.
            totalresolvedips++;
            //pthread_mutex_unlock(&lock);
            hostptr[index] = (char *)malloc(256);

The threads will become useless but if you get the correct answer, you know where the problem is.
I will see if I can find a corresponding thread safe method.
I started with Experts Exchange in 2004 and it's been a mainstay of my professional computing life since. It helped me launch a career as a programmer / Oracle data analyst
William Peck
avsrivastava

try getnameinfo() function instead, it does not use any static structues and is thread safe (on linux platforms atleast).
it has a bit different syntax.
its inverse is the getaddrinfo function.
avsrivastava

Hey, did you manage it? any new trouble?
summer_soccer

ASKER
I tried to use getnameinfo(), but it took me some time trying to make the program work. Finally I gave up. I already could not remember what is wrong.

Can you paste some example code of getnameinfo()? It seems this function requires a parameter of struct sockaddr. I only know how to use the struct sockaddr_in.
Get an unlimited membership to EE for less than $4 a week.
Unlimited question asking, solutions, articles and more.
ASKER CERTIFIED SOLUTION
avsrivastava

Log in or sign up to see answer
Become an EE member today7-DAY FREE TRIAL
Members can start a 7-Day Free trial then enjoy unlimited access to the platform
Sign up - Free for 7 days
or
Learn why we charge membership fees
We get it - no one likes a content blocker. Take one extra minute and find out why we block content.
Not exactly the question you had in mind?
Sign up for an EE membership and get your own personalized solution. With an EE membership, you can ask unlimited troubleshooting, research, or opinion questions.
ask a question
summer_soccer

ASKER
Thank you very much, avsrivastava. Now the program works way faster than before, and gets the correct results. Yes, getnameinfo() is thread-safe.