[C++] Socketprobleem, kan niet connecten over inet

[Verwijderd]

Ik heb een programma gemaakt dat moet luisteren naar inkomende verbindingen over internet/netwerk, als ik via m'n netwerk verbinding maak met de 'server' werkt alles naar behoren van zodra ik echter vanop een andere computer met m'n 'server' wil connecten over internet loopt het fout. Er gebeurt dan helemaal niets er wordt geen verbinding gemaakt met het programma, het lijkt dus alsof het helemaal niet 'luistert' naar verbindingen inkomende via het internet.
Ik dacht eerst aan m'n firewall ofzo maar daar blijkt het niet aan te liggen, iemand een idee ?

bvd,

C++:

int main(void) 
{ 
    //Initialiseer winsock 
    WSADATA wsaData; 
    if(WSAStartup(MAKEWORD(1,1), &wsaData)) { 
        cout << "\nWSAStartup faalde."; 
        exit(1); 
    } 

    //Nodig voor CreateThread 
    DWORD dwThreadId;  
    HANDLE hThread; 
     
    struct sockaddr_in my_addr, their_addr; 
    char yes = '1';  
    int sin_size, socketfd, newsocketfd; 
    struct hostent *client_info; 

    if( (socketfd = socket(AF_INET, SOCK_STREAM, 0)) == -1 ) { 
        cout << "\nsocket() faalde."; 
        exit(1); 
    } 

    if( setsockopt(socketfd, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(char)) == -1 ) { 
        cout << "\nsetsockopt() faalde."; 
        exit(1); 
    } 

    my_addr.sin_family = AF_INET; 
    my_addr.sin_port = htons(MYPORT); 
    my_addr.sin_addr.S_un.S_addr = htonl(INADDR_ANY); 
    memset(&(my_addr.sin_zero), '', 8); 

    if( bind(socketfd, (struct sockaddr *)&my_addr, sizeof(struct sockaddr)) == -1 ) { 
        cout << "\nbind() faalde."; 
        exit(1); 
    } 
     
    if( listen(socketfd, BACKLOG) == -1 ) { 
        t>cout << "\nlisten() faalde."; 
        exit(1); 
    } 

    while(TRUE) { //accept loop 
        sin_size = sizeof(struct sockaddr_in); 
         
        if( (newsocketfd = accept(socketfd, (struct sockaddr *)&their_addr, &sin_size)) == -1 ) { 
            cout << "\naccept() faalde."; 
            continue; //server mag niet down gaan door een foute accept() 
        } 
         
        client_info = gethostbyaddr((const char *)&their_addr.sin_addr, sizeof(struct in_addr), AF_INET);  
        cout << "\nConnected with " << client_info->h_name <<" (" << inet_ntoa(their_addr.sin_addr) << ")"; 
         
        if( !(hThread = CreateThread(NULL, 0, handleRequest, &newsocketfd, 0, &dwThreadId)) ) { 
            cout << "CreateThread() faalde."; 
            exit(1); 
        } else { 
            CloseHandle(hThread); 
        } 
    } 
     
    closesocket(socketfd); 
    WSACleanup(); 

    return 0; 
}

[madwizard]

Volgens mij hoeft INADDR_ANY niet door htonl gehaald te worden, al maakt het niet uit want INADDR_ANY==0. Verder heb ik geen problemen met deze code, kan de server zowel lokaal als van buitenaf bereiken..

[Verwijderd]

Je moet hem niet door htonl() halen, maar ik haal hem er toch door omdat er mss OS's zijn die het niet in network byte order doorgeven. Is gewoon om bij het porten zo min mogelijk problemen te hebben (moet enkel die createthread dan nog vervangen en mss die char yes).

Bij jou werkt het dus Raar ... dan ligt het toch aan mijn computer

[curry684]

Even je code reviewen hoor:

C++:

    struct sockaddr_in my_addr, their_addr; 

    // Zou ik schrijven als:
    struct sockaddr_in my_addr = {0}, their_addr = {0};

Je weet niet hoe groot de structuur sockaddr_in ooit nog gaat worden (hij verschilt zelfs per platform!), dus gewoon als geheel op nul initialiseren is het best.

C++:

    if( (socketfd = socket(AF_INET, SOCK_STREAM, 0)) == -1 ) { 

Wat is die derde parameter? Ik neem aan dat je hier de constante IPPROTO_TCP bedoelt? Wederom: denk aan je portability, en dan ook vooral de portability richting toekomst zoals IPv6 winsock extensions!

C++:

    if( setsockopt(socketfd, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(char)) == -1 )

Weinig zin om PSDK uit de spitten, maar mijn werkende TCP/UDP-serverimplementatie zet deze optie niet. Daarentegen zet ik wel SO_KEEPALIVE op true en SO_LINGER op {true, 30}.

C++:

    my_addr.sin_family = AF_INET; 
    my_addr.sin_port = htons(MYPORT); 
    my_addr.sin_addr.S_un.S_addr = htonl(INADDR_ANY); 
    memset(&(my_addr.sin_zero), '', 8); 

Die laatste memset is dus een 'backwards compatible' actie. Je initialiseert een member waarvan je weet dat hij nu bestaat op nullen, en je gokt dat hij 8 bytes is. Op z'n minst had daar een sizeof(sockaddr_in) moeten staan, maar doordat we zoals ik hierboven zei de hele structuur op nul zouden moeten initialiseren is dit overbodig.

C++:

    if( bind(socketfd, (struct sockaddr *)&my_addr, sizeof(struct sockaddr)) == -1 ) { 

Iedere non-NULL value is foute boel, dus hier moet != 0 staan.

C++:

    if( listen(socketfd, BACKLOG) == -1 ) { 

De errorcode van listen is de constante SOCKET_ERROR. Check daarop, en niet op -1.

C++:

        if( (newsocketfd = accept(socketfd, (struct sockaddr *)&their_addr, &sin_size)) == -1 ) { 
            cout << "\naccept() faalde."; 
            continue; //server mag niet down gaan door een foute accept() 
        } 

Foute accepts? Die bestaan niet, tenminste niet als je select gebruikt zoals het hoort. Select kan tevens timeouts aan zodat je niet multithreaded hoeft te werken als je niet perse wil (poll-based networking).

Als na een succesvolle select de accept failed heb je overigens een doodgeboren socket, wat wel een fatal error is!


Op de dingen na die ik hier heb aangestipt ziet je code er perfect uit. Tzal dus wel je firewall zijn.

[ Voor 39% gewijzigd door curry684 op 19-02-2003 23:49 . Reden: Oepsie misclick ]

[Verwijderd]

Elke functie kan foutlopen hé, dus ik controleer op accept(). Meestal komt het niet voor maar het kan, anders hadden ze geen returnwaarde geïmplementeerd.

De bedoeling van het programa is dat hij meerdere requests tegelijk kan afwerken, dus lijken threads me de beste methode.

Over die foutafhandeling moet ik je wel gelijk geven, ik ga die SOCKET_ERROR gebruiken ipv. -1

[curry684]

Verwijderd schreef op 20 February 2003 @ 12:42:
Elke functie kan foutlopen hé, dus ik controleer op accept(). Meestal komt het niet voor maar het kan, anders hadden ze geen returnwaarde geïmplementeerd.

De bedoeling van het programa is dat hij meerdere requests tegelijk kan afwerken, dus lijken threads me de beste methode.

Ik zei dus:

Foute accepts? Die bestaan niet, tenminste niet als je select gebruikt zoals het hoort. Select kan tevens timeouts aan zodat je niet multithreaded hoeft te werken als je niet perse wil (poll-based networking).

Als na een succesvolle select de accept failed heb je overigens een doodgeboren socket, wat wel een fatal error is!

Oftewel: als je select gebruikt hoef je niet multithreaded als je niet wil, je kunt je dan namelijk een hoop geemmer met synchronization en alle risico's van dien besparen door zelf multithreading te faken. Hangt er vanaf wat je ermee wil doen, als je maar hooguit een paar milliseconden per request bezig bent zou ik multithreading laten zitten. Zelfs voor een chatserver met duizenden bezoekers kan het met select beter werken dan met multithreading!

Daarnaast kondigt select aan wanneer een accept succesvol gaat zijn. Pas zodra na die succesvolle select gemeld wordt dat accept fout gaat is er stront aan de knikker.

Ik zeg niet dat je het niet multithreaded of zonder select mag doen, ik zeg dat het opties zijn die zeker het overwegen waard zijn afhankelijk van wat je ermee wil doen. Les 1 van goed programmeren: staar je niet blind op de oplossing die je vantevoren voor ogen staat.

[Zoijar]

curry684 schreef op 20 februari 2003 @ 22:09:
. Zelfs voor een chatserver met duizenden bezoekers kan het met select beter werken dan met multithreading!

Dat werkt zelfs beter. Je kan niet zo maar duizend threads creeren zonder consequenties. Die moeten allemaal ge-scheduled worden etc. Threading is helemaal niet zo'n fijne oplossing. Je kan beter een connection thread en een "process" thread maken. De connection thread doet accept() en voegt de nieuwe connectie toe aan een lijst oid (met lock). En de process thread handeld alle connecties stuk voor stuk af.

[curry684]

[rml][ win32/alg] mutual read/write exclusion op veel objecten[/rml]
[rml]curry684 in "[ win32/alg] mutual read/write exclusion "[/rml]

Wink

[Verwijderd]

Zoijar schreef op 21 February 2003 @ 02:16:
[...]


Dat werkt zelfs beter. Je kan niet zo maar duizend threads creeren zonder consequenties. Die moeten allemaal ge-scheduled worden etc. Threading is helemaal niet zo'n fijne oplossing. Je kan beter een connection thread en een "process" thread maken. De connection thread doet accept() en voegt de nieuwe connectie toe aan een lijst oid (met lock). En de process thread handeld alle connecties stuk voor stuk af.

offtopic:
ehhh, klinkt als .... IO completion ports, maar goed dat beantwoord je vraag niet.

Dit lijkt al met al niet zozeer een code probleem te zijn, als wel een netwerk probleem. Je zou bijvoorbeeld eens kunnen testen of je wel van buiten op een locale ftp/telnet/whatever server kan connecten.

[Verwijderd]

Ik kan van buitenaf met m'n lokale ftp, telnet, ssh, web -server connecten.

De bedoeling is een (bescheiden) webserver te ontwerpen de request wordt nu telkens een thread afgehandeld, omdat ik dacht dat dat beter werkte dan een select, maar als jij zo overtuigd spreekt over select() zou ik mss beter met select werken dan ....

Over die les één in goed programmeren -> ik staar me heus niet blind op de oplossing die ik voor ogen had, ik vond (en vind tot nog toe) threads de beste oplossing, maar zoals ik net zei ik zal wel eens kijken of ik het mss beter met select zou doen.

[madwizard]

Winsock met I/O completion ports is waarschijnlijk de meest efficiente oplossing (in ieder geval de meest schaalbare), maar is tenzij je met meer dan 10.000 verbindingen gaat werken waarschijnlijk overkill. Een thread per client is oke als er niet al te veel clients zijn, threads zijn niet echt goedkope resources. Zelfs met I/O completion is de ideale configuratie 1 thread per CPU, aangenomen dat er geen blocking operaties worden uitgevoerd.

Het vervelende van select is dat je weinig controle hebt zolang de call blockt als er geen events zijn. Je kunt uiteraard wel een timeout gebruiken om tussendoor andere dingen te checken (bijvoorbeeld of de server gesloten moet worden) maar dan krijg je een soort van polling-idee (alleen om de zoveel tijd).

Een snel alternatief is WSAEventSelect, die een event object triggert als er een netwerk event gebeurd. Met WaitForMultipleObjects/WSAWaitForMultipleEvents kun je dan niet alleen op netwerk events maar ook op je eigen events controleren zodat je wat meer controle hebt.

Als portability een belangrijk punt is is select denk ik het makkelijkst.

edit: Hou er ook rekening mee dat select (in ieder geval in windows) een limit heeft van 64 sockets waar je de status van kan controleren. Bij meerdere sockets moet je dus of meerdere threads gebruiken (64 verbindingen per thread) of een timeout en select om beurten aanroepen op de verschillende series sockets.

[ Voor 14% gewijzigd door madwizard op 22-02-2003 00:53 ]

[Verwijderd]

Ah oké, bedankt voor je uitleg. Ik ga dan maar met select() aan de slag gaan, en winsock met I/O completion ports bekijk ik ook eens mocht ik meer dan 10.000 verbindingen nodig hebben

iig. 'iedereen' bedankt voor jullie uitleg

[farlane]

curry684 schreef op 19 februari 2003 @ 23:42:
Even je code reviewen hoor:

C++:

    if( setsockopt(socketfd, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(char)) == -1 )

Weinig zin om PSDK uit de spitten, maar mijn werkende TCP/UDP-serverimplementatie zet deze optie niet. Daarentegen zet ik wel SO_KEEPALIVE op true en SO_LINGER op {true, 30}.

SO_REUSEADDR
By default, a socket cannot be bound (see bind) to a local address that is already in use. On occasion, however, it can be necessary to reuse an address in this way. Since every connection is uniquely identified by the combination of local and remote addresses, there is no problem with having two sockets bound to the same local address as long as the remote addresses are different. To inform the Windows Sockets provider that a bind on a socket should not be disallowed because the desired address is already in use by another socket, the application should set the SO_REUSEADDR socket option for the socket before issuing the bind. The option is interpreted only at the time of the bind. It is therefore unnecessary and harmless to set the option on a socket that is not to be bound to an existing address. Setting or resetting the option after the bind has no effect on this or any other socket.