C Windows winapi + multi-threading (HANDLE)_beginthreadex fail

[exitthematrix]

This code ruined all my day. Basically I have a list of 50 webservers I administrate, I want to check them if up/alive (isAlive() function), I parse my webservers.txt file with the 50 ips/hostnames and for fastness I try to use threads (10, 20 or 30 doesn't matter) then my compiled code seems only duplicate threads with the last line in my file.

#include <windows.h>
#include <stdio.h>
#include <string.h>
#include <winsock.h>
#include <process.h>
#include <string.h>

#pragma comment(lib, "wsock32.lib")

unsigned int _stdcall isAlive(void *ptr)
{
  struct sockaddr_in blah;
  struct hostent *he;
  WSADATA wsaData;
  int i;
  WORD wVersionRequested;
  SOCKET sock;
  char* addr = (char*)ptr;

  char buff[1024];
  char *ex;
  ex="GET /alive.php HTTP/1.0\n\n";
  char *fmsg="ALIVE";

  wVersionRequested = MAKEWORD(1, 1);
  if (WSAStartup(wVersionRequested , &wsaData)){
    printf("Winsock Initialization failed.\n");
    return(1);
  }

  if ((sock=socket(AF_INET,SOCK_STREAM,0))==INVALID_SOCKET){
    printf("Can not create socket.\n");
    return(1);
  }
  sock = socket(AF_INET,SOCK_STREAM,0);
  blah.sin_family = AF_INET;
  blah.sin_port = htons(80);
  blah.sin_addr.s_addr = inet_addr(addr);

  if ((he=gethostbyname(addr))!=NULL){
    memcpy((char *)&blah.sin_addr.s_addr,he->h_addr,he->h_length);
  }
  else{
    if((blah.sin_addr.s_addr=inet_addr(addr))==-1){
      WSACleanup();
      return(1);
    }
  }

  if (connect(sock,(struct sockaddr*)&blah,sizeof(blah))==0){
    send(sock,ex,strlen(ex),0);
    recv(sock,buff,sizeof(buff),0);
    if(strstr(buff,fmsg)!=NULL){
      printf("ALIVE: %s\n", addr);
    }
  }

  closesocket(sock);
  WSACleanup();
  _endthreadex(0);
  return(1);
}

int main(int argc,char *argv[])
{
  if(argc!=2){
    printf("Usage: %s <webservers list>\n", argv[0]);
    return(1);
  }

  char *inname = argv[1];
  FILE *infile;
  char line_buffer[BUFSIZ];
  char line_number;

  infile = fopen(inname, "r");
  if (!infile) {
    printf("Couldn't open file %s for reading.\n", inname);
    return 0;
  }

  line_number = 0;
  HANDLE hThreadArray[200];
  while (fgets(line_buffer, sizeof(line_buffer), infile)) {
    unsigned threadID;
	hThreadArray[line_number] = (HANDLE)_beginthreadex(0, 0, isAlive, line_buffer, 0, &threadID);
    ++line_number;
  }
  WaitForMultipleObjects(line_number, hThreadArray, TRUE, INFINITE);
  fclose(infile);
  return 0;
}

hosts.txt (11 lines in it)

myhost.com
mysecondhost.com
...
mylasthost.com

result:

C:\Documents and Settings\Xtmtrx\Desktop\Code>checkalive.exe hosts.txt
ALIVE: mylasthost.com
ALIVE: mylasthost.com
ALIVE: mylasthost.com
ALIVE: mylasthost.com
ALIVE: mylasthost.com
ALIVE: mylasthost.com
ALIVE: mylasthost.com
ALIVE: mylasthost.com
ALIVE: mylasthost.com
ALIVE: mylasthost.com
ALIVE: mylasthost.com

[D_Drmmr]

  char line_buffer[BUFSIZ];
  char line_number;

  /* ... */

  line_number = 0;
  HANDLE hThreadArray[200];
  while (fgets(line_buffer, sizeof(line_buffer), infile)) {
    unsigned threadID;
	hThreadArray[line_number] = (HANDLE)_beginthreadex(0, 0, isAlive, line_buffer, 0, &threadID);
    ++line_number;
  }
  WaitForMultipleObjects(line_number, hThreadArray, TRUE, INFINITE);
  fclose(infile);
  return 0;
}

You are passing a pointer to the 'line_buffer' to each thread. However, you don't know when that thread will start. You may well have overwritten the contents of the buffer with the next line before the thread starts.

[exitthematrix]

An how should I safely start every thread with different lines read from my file? Because if I just do "printf("%s", line_buffer)" that works.

[D_Drmmr]

An how should I safely start every thread with different lines read from my file? Because if I just do "printf("%s", line_buffer)" that works.

What do you mean with "that works"? Doing a single run and looking at the results does not tell you much in a multi-threaded world.

You should use a different buffer for each thread. That way you don't need to perform any synchronization. Seems the easiest for the use case you described.

[exitthematrix]

What do you mean with "that works"? Doing a single run and looking at the results does not tell you much in a multi-threaded world.

You should use a different buffer for each thread. That way you don't need to perform any synchronization. Seems the easiest for the use case you described.

By "that works" I mean using "printf("%s", line_buffer)" that prints out every line out from my text.

[superbonzo]

as suggested by D_Drmmr, a minimal code change would be

char line_buffer[200][BUFSIZ];
char line_number;

/* ... */

line_number = 0;
HANDLE hThreadArray[200];
while (fgets(line_buffer[line_number], sizeof(line_buffer[line_number]), infile)) {
  unsigned threadID;
  hThreadArray[line_number] = (HANDLE)_beginthreadex(0, 0, isAlive, line_buffer[line_number], 0, &threadID);
  ++line_number;
}
WaitForMultipleObjects(line_number, hThreadArray, TRUE, INFINITE);

// ...

( ignoring issues like possible buffer overruns, unchecked retun codes, etc ... )

alternatively, you could just pass the file path and a liner number to each thread, that in turn will open its own file stream seeking to the appropriate line.

[D_Drmmr]

By "that works" I mean using "printf("%s", line_buffer)" that prints out every line out from my text.

There's little point in being vague if you want to get help. At the very least, you could post the code.
What you have now is a race condition, because you are writing (in the main thread) to the same memory you are reading from in the worker threads. That means that the results could be different on each run of your program.

[exitthematrix]

Thanks!

I did it like this:

	HANDLE hThreadArray[200];
  	char str[MAX];
  	char *x[MAX];
  	int i = 0;
    while(!feof(infile)) {
        while(fgets(str, sizeof str, infile)) {
        	unsigned threadID;
            x[i] = strdup(str);
            //printf("%s", str);
            //printf("%s", *(x+i));
            hThreadArray[i] = (HANDLE)_beginthreadex(0, 0, isAlive, *(x+i), 0, &threadID);
            i++;
        }
	}
	
  WaitForMultipleObjects(i, hThreadArray, TRUE, INFINITE);

[exitthematrix]

This is my finished code, I added timeout also but seems to hang connected if no response back, otherwise works:

#include <windows.h>
#include <stdio.h>
#include <string.h>
#include <winsock.h>
#include <process.h>
#include <string.h>

#pragma comment(lib, "wsock32.lib")

#define MAX 10000

unsigned int _stdcall isAlive(void *ptr)
{
  struct sockaddr_in blah;
  struct hostent *he;
  WSADATA wsaData;
  int i;
  WORD wVersionRequested;
  SOCKET sock;
  char* addr = (char*)ptr;

  char buff[1024];
  char *request;
  request="GET /alive.php HTTP/1.0\n\n";
  char *fmsg="ALIVE";

  wVersionRequested = MAKEWORD(1, 1);
  if (WSAStartup(wVersionRequested , &wsaData)){
    printf("Winsock Initialization failed.\n");
    return(1);
  }

  if ((sock=socket(AF_INET,SOCK_STREAM,0))==INVALID_SOCKET){
    printf("Can not create socket.\n");
    return(1);
  }
  sock = socket(AF_INET,SOCK_STREAM,0);
  blah.sin_family = AF_INET;
  blah.sin_port = htons(80);
  blah.sin_addr.s_addr = inet_addr(addr);

  if ((he=gethostbyname(addr))!=NULL){
    memcpy((char *)&blah.sin_addr.s_addr,he->h_addr,he->h_length);
  }
  else{
    if((blah.sin_addr.s_addr=inet_addr(addr))==-1){
      WSACleanup();
      return(1);
    }
  }

  if (connect(sock,(struct sockaddr*)&blah,sizeof(blah))==0){
    send(sock,request,strlen(request),0);
    recv(sock,buff,sizeof(buff),0);
    if(strstr(buff,fmsg)!=NULL){
      printf("ALIVE: %s", addr);
    }
  }

  closesocket(sock);
  WSACleanup();
  _endthreadex(0);
  return(1);
}

int main(int argc,char *argv[])
{
  if(argc!=2){
    printf("Usage: %s <webservers list>\n", argv[0]);
    return(1);
  }

  char *inname = argv[1];
  FILE *infile;
  char line_buffer[BUFSIZ];
  char line_number;

  infile = fopen(inname, "r");
  if (!infile) {
    printf("Couldn't open file %s for reading.\n", inname);
    return 0;
  }

  HANDLE hThreadArray[200];
  char str[MAX];
  char *x[MAX];
  int i = 0;
  while(!feof(infile)) {
    while(fgets(str, sizeof str, infile)) {
      unsigned threadID;
      x[i] = strdup(str);
      //printf("%s", *(x+i)); // DEBUG
      hThreadArray[i] = (HANDLE)_beginthreadex(0, 0, isAlive, *(x+i), 0, &threadID);
      i++;
      }
  }
	
  WaitForMultipleObjects(i, hThreadArray, TRUE, INFINITE);
  fclose(infile);
  return 0;
}