マルチスレッドのエコーサーバーの不具合

[shiro4ao]

WinsockAPIを用いた、マルチスレッド実装のエコーサーバーを
つくろうとしています。

メインスレッドはaccept()し、サブスレッド(Thread2)で読み込み可能なソケットを
探し（GetReadableSock関数）、クライアントの相手をしています。
サブスレッドはクライアントからのデータをそのまま帰すだけです。


問題点はたくさんあると思いますが、主に２点です

・新しいクライアントが接続しても、先に接続していた他のクライアントが送信するまで
　送信したデータを処理してくれない
　　→GetReadableSock()内でのselect関数が新しいソケットの面倒を見てくれない為だと思われる
　　→GetReadableSock()をどう変更した方がいいかわからない

・例えばクライアントが２人いた場合に、先に接続したクライアントが、切断すると
　GetReadableSock()内のselect()がエラーを返す。
　　→切断したクライアントのソケットが登録されたままになっているためだと思われる
　　→GetReadableSock()内でINVALID_SOCKETを登録しないようにしてみましたが問題解決で
　　　きませんでした


何か良い方法はあるでしょうか？
それとも根本からおかしいのでしょうか？

コード:

 
#include    <stdio.h>
#include    <conio.h>
#include    <winsock2.h>

int nClient;
SOCKET  ClientSock[10];
HANDLE hThread, hEvent;

//読み込み可能なソケットを返す
int GetReadableSock( unsigned int s[] ,int N)
{
  int i;
  fd_set fdset;

  // fd_set変数を0に初期化する
  FD_ZERO( &fdset );

  // 各ソケットの番号を登録する
  for( i=0; i<N; i++ ){
    if(s[i]!=INVALID_SOCKET)FD_SET( s[i], &fdset );  //INVALID_SOCKETだったら登録しない・・・
  }

  // ソケット番号の最大値を探す
  int max_s=0;
  for( i=0; i<N; i++ ){
    if( max_s < s[i] ) max_s = s[i];
  }

  // タイムアウトを10秒に設定
  struct timeval tv;
  tv.tv_sec  = 7000;
  tv.tv_usec =  0;

  // ソケットにデータが届くまで待つ
  int n = select( max_s+1, &fdset, NULL, NULL, &tv );

  if( n == -1 ){ // 何らかのエラー
    perror("select");
    return -1;
  }else if( n == 0 ){ // タイムアウト
    fprintf( stderr, "select timeout\n" );
    return 0;
  }else if( n>0 ){ // どれかにデータが届いた
    // どれが読み込み可能か検査
    for( i=0; i<N; i++ ){
      if( FD_ISSET( s[i], &fdset ) ){
	return s[i]; // 読み込めるソケットを返す
      }
    }
    return 0;
  }
  return 0;
}


DWORD WINAPI Thread2(LPVOID lpvoid ) {
	char buf[4000];
	int ret,n,i;
	printf("ThreadOpened\n");
	while(1){
		WaitForSingleObject(hEvent,INFINITE);
		SOCKET s=GetReadableSock(ClientSock,nClient);
                        printf("s=%d\n",s);
		memset(buf,'\0',sizeof(buf));
		ret=recv(s,buf,1024,0);

		if(ret>0){
			printf("%s\n",buf);
			send(s,buf,ret,0);
		}
		if(ret==0){
			closesocket(s);
			nClient--;
			s=INVALID_SOCKET;
			if(nClient==0)ResetEvent(hEvent);
                               printf("nClient=%d\n",nClient);
		}

	}

	ExitThread(TRUE);
}


int main(void)
{ 
    WSADATA wsaData;
    SOCKET  ServSock,readable;
    struct  sockaddr_in addr;
    struct  sockaddr_in client;
    struct  sockaddr_in client2;
    int     len,i=0,ret;
    char buf[1024],c;
    DWORD dwParam1;

    nClient=0;

      WSAStartup(MAKEWORD(2,0), &wsaData);
   
    ServSock= socket(AF_INET,SOCK_STREAM,0);

    addr.sin_family = AF_INET;
    addr.sin_port = htons(5555);
    addr.sin_addr.S_un.S_addr = INADDR_ANY;
    bind(ServSock, (struct sockaddr *)&addr, sizeof(addr));
    printf("接続待ちです\n");

    listen(ServSock, 10);

    for(i=0;i<10;i++)ClientSock[i]=INVALID_SOCKET;


    len = sizeof(client);
    hThread=CreateThread(NULL , 0 , Thread2 ,NULL , CREATE_SUSPENDED , &dwParam1);
    hEvent=CreateEvent(NULL, TRUE, FALSE, "EV01");
    if(hEvent==NULL)MessageBox(NULL,"","",NULL);
    ResumeThread(hThread);

    while(nClient<10){
        len = sizeof(client);
        ClientSock[nClient] = accept(ServSock, (struct sockaddr *)&client, &len);
        nClient++;
        if(nClient==1)SetEvent(hEvent);
        printf("Accepted=%d\n",nClient);
    }

    for(i=0;i<10;i++)closesocket(ClientSock[i]);
    closesocket(ServSock);
    WSACleanup();
    return 0;
}

[Poco]

1つのスレッドで複数のリクエストを処理しようとしていますが、これは本当に意図した仕様でしょうか？
シンプルに要求を受け付けるたびにスレッドを作成する、すなわち、1つのスレッドで1つのリクエストを処理するようにしてみてはどうですか？

>新しいクライアントが接続しても、先に接続していた他のクライアントが送信するまで
>　送信したデータを処理してくれない
>　　→GetReadableSock()内でのselect関数が新しいソケットの面倒を見てくれない為だと思われる

というより、GetReadableSock()が「古いソケットから順番に面倒みようとしている」からでは？
一度処理したソケットは、配列の最後に回したりしてみてはどうですか？

>例えばクライアントが２人いた場合に、先に接続したクライアントが、切断すると
>GetReadableSock()内のselect()がエラーを返す。

selectの第3引数を設定して、エラーが発生した場合に、エラーが発生したソケットを特定して、
除外後に、selectをリトライしてみてはどうですか？
＃↑は自信なしです。この手のプログラム組んだのは昔の話なので・・・＾＾；

[Ciel]

私もコンソールアプリケーションでサーバを作ろうとして、一つのクライアントが接続するごとに、
スレッドを作って、ポーリングして対応してました。

クライアントからの受信はそれで済みましたが、サーバから送信するとなると、またブロッキングの対応をしなければならなかったりして、
すごい効率がわるかったので、イベントを受信して対応するようにしました。

私はサーバ側はWSAAsyncSelectを使ってノンブロッキングモードにし、全てウィンドウメッセージで受信して処理するようにしました。
これはGUIアプリじゃないと使えない関数ですが、この関数のおかげで多対多のチャットをほぼ完成させることができました。
受信できるとき、接続してきたときにイベントが発生して、ウィンドウメッセージで通知してくれるので、
その時に処理させるだけです。
すごい簡単ですので一度やってみてはいかがでしょうか？

WSAAsyncSelectはGUI専用ですが、コンソールアプリ用のWSAEventSelectという、ほぼ同じ機能を持った関数があるようです。
こっちの関数は使ったことはありませんが、一度ググってみると幸せになれるかもしれません。

適当に書きましたが、参考になれば幸いです。

[shiro4ao]

＞ぽこさん
　ありがとうございます。
＞1つのスレッドで1つのリクエストを処理する
　１クライアント１スレッドにしようと思ったのですが、
　クライアントが７０名程になることも視野に入れ、なるべくリソースを消費しないよう
　な実装を考えていました。しかし、試してみようと思います。

　追記：select()自体にも６４ソケット制限があるようで、１クライアント１スレッド
　　　　が適切のようです。（もしくは１スレッドあたりで６４個を超えないようにする）

＞一度処理したソケットは、配列の最後に回したりしてみてはどうですか？
　この部分がよくわかりませんでした、多分自分がselect()をよく理解していないためだと思います。

＞エラーが発生したソケットを特定して、除外後に、selectをリトライしてみてはどうですか？
　エラーのでたソケットを除去したいのですが、GetReadableSock()の

コード:

　  // 各ソケットの番号を登録する
    for( i=0; i<N; i++ ){
       if(s[i]!=INVALID_SOCKET)FD_SET( s[i], &fdset );  //INVALID_SOCKETだったら登録しない・・・
    }

　で、除去できると思っていたのです（切断したソケットにはINVALID_SOCKETをいれている）が、
　おかしいのでしょうか？自分はFD_SETマクロの挙動が理解がよく出来ていないようです



＞Cielさん
　ありがとうございます。
　WSAAsyncSelect()は６４ソケット制限（内部でWaitForMultipleObjects()を利用していて、それは
　６４個までのイベントオブジェクトしか持てない）を抱えているため、７０名のクライアントの
　面倒を見たいサーバーでは色々と問題があり、マルチスレッドという方法にいきあたりました。

　ユーザーインターフェースインフラストラクチャは既に存在しているのでイベントを利用した方　
　が早いと思いますが
　スレッドごとにウィンドウプロシージャを割り当てる方法がわからなかったため、マルチスレッド
　での実装にしました。WSAEventSelect（）を少し考えてみようと思います。

[shiro4ao]

なんとか、１クライアント１スレッドで実装することができました。
余力があれば、イベントオブジェクトを用いた方も実験してみよう
と思います。
ありがとうございました。