C++: Segmentation fault in meinem WebsocketServer

[Daniel A. (daniel-a)]

Für eine grosse Webseite, welche ich als Privatprojekt am entwickeln 
bin, brauche ich einen Server, welcher das HTTP und das Websocket 
Protokoll unterstützt. Aufgrund grösserer Flexibilität habe ich mich 
entschieden einen eigenen WebsocketServer in C++ zu programmieren. 
Dieser funktioniert auch bereits teilweise, aber leider kommt es beim 
aufrufen einer Seite manchmal zu einem Segmentation fault. Ich konnte 
leider noch nicht herausfinden, was zu diesem Segmentation fault führt, 
und hoffe hier auf Tipps, Hinweise, Kritik und/oder 
Verbesserungsvorschläge.

Den Sourcecode gibt es hier:
  http://87.102.188.235/WebsocketServer.zip

GDB hat mir folgendes verraten:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff6e92700 (LWP 5005)]
0x00000000004052d0 in client::ws_send(void*) ()
(gdb) backtrace
#0  0x00000000004052d0 in client::ws_send(void*) ()
#1  0x00000000004059a5 in task::solve(unsigned char*) ()
#2  0x0000000000402eca in c_worker::do_work() ()
#3  0x0000000000402ff2 in worker() ()
#4  0x00000000004026b0 in callThreadFunc(void*) ()
#5  0x00007ffff7bc7b50 in start_thread (arg=<optimized out>) at pthread_create.c:304
#6  0x00007ffff6f6f0ed in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#7  0x0000000000000000 in ?? ()

[Dr. Sommer (Gast)]

Und jetzt erwartest du dass sich jemand gratis den Kopf für dich 
zerbricht und dein Programm auseinander nimmt um deinen Fehler zu 
finden?

Das darfst du schön selber machen. Kompiliere & Linke mit "-g" um 
bessere debug-Informationen zu erhalten. Schaue in GDB nach wo genau der 
Fehler auftritt. Finde heraus was den Fehler verursacht. Verwende 
"valgrind" um Speicherfehler zu finden ( Fehler die nicht sofort zum 
Absturz führen ).

[Dr. Sommer (Gast)]

void forEachFile(const char* path,void(*func)(const char*,const char*)){

    DIR *dp = opendir(path);
    struct dirent *dirp;

    if(!dp) {
      printf("error: failed to opendir \"%s\"",path);
      return;
    }

    char* p = (char*)malloc(1024);
    while( (dirp=readdir(dp)) ){
      if(!dirp->d_name||dirp->d_name[0]=='.'||!dirp->d_name[0])
        continue;
      sprintf(p,"%s/%s",path,dirp->d_name);
      (*func)(p,m_basename(dirp->d_name));
    }
    free(p);

    closedir(dp);

    return;
}

  FILE* f = fopen(path,"rb");
  if(!f){
    printf("error: cann't open file %s\n",path);
    redirect(buff,"/");
    return false;
  }

        for(unsigned j=0;j<header_field_list_length;j++){
          if(!strcmp(header_field_list_names[j],(char*)&buff[n])){

Das nennst du C++ Code?! Das sieht eher nach "schönstem" C aus.

    c_worker** _workers = (c_worker**)realloc(workers,(i+1)*sizeof(c_worker*));

Was ist das?! Schonmal von std::vector gehört?

  new server("0.0.0.0",80);

Speicher aufräumen, braucht ja keiner?!

etc. etc. ...

[Peter II (Gast)]

der code ist furchtbar!

Wenig C++ - viel C - viele mögliche fehler!

>  str = (char*)realloc(str,s);
>  str[s-1]=0;

was ist wenn kein Speicher da ist?

> #include "c_worker.cpp"
wer macht denn so etwas?

auch sollte man wenn man schon C++ macht eine string lib benutzen. Dann 
könnte man sich soetwas unlesbares sparen:

> for(a=str+s-1;a>=str  &&(*a==' '||*a==0);a--);

Da ist überhaupt keine sinnvolle Struktur erkennbar.

Warum wird das ganze http Protokoll in Client.cpp zerlegt? Dafür 
schreibt man sich eine http Klasse.

Auch für die Socket sollte man an anfang gleich eine Klasse oder eine 
fertige C++ lib nutzten. So verliert man doch alle Vorteile der 
Fehlerbehandlung von C++

die http Header sind ideal für eine std::map - aber hier wird wieder 
manuell mit listen gearbeitet.


Mit diesen Code würde ich keine Webserver laufen lassen, hier ist es nur 
eine Frage der Zeit bis jemand von außen mit einen Buffer Overflow das 
System lahmlegt oder gleich übernimmt.

Ein fehler wird hier nicht so schnell jemand finden.

[g457 (Gast)]

Ich tippse auf out-of-Memory - hab mindestens ein leak gefunden [0] das 
gefixt werden will.

Und jetzt wieder zurück zur Glaskugel.

[0] spoiler: [1]
[1] c_worker::buff

[Daniel A. (daniel-a)]

g457 schrieb:
> hab mindestens ein leak gefunden [0] das
> gefixt werden will.
...
> [1] c_worker::buff

Das ist kein leak, weil:
  a) c_worker::buff nur einmal, bei der Initialisierung speicher 
reserviert
  b) Immer benötigt wird
  c) Das Programm nie beendet wird

[Dr. Sommer (Gast)]

Dann hoffen wir mal, dass auch in zukünftigen Versionen niemals später 
Worker herausgenommen werden. Denn dann musst du dich ja daran erinnern 
(mit Post-It am Monitor zB) den Speicher dann doch freizugeben.
Alternativ kann man natürlich auch sauberen Code schreiben der auch 
später noch wiederverwendet/modifiziert werden kann:

#include <memory>

static constexpr BUFF_MAX = 1024; // #define ist gefährlich und unnötig, besser Konstanten verwenden
class c_worker {
  std::unique_ptr<unsigned char[]> buff;
...
c_worker::c_worker () {
  buff = std::unique_ptr<unsigned char[]> (new unsigned char [BUFF_MAX]);
}

Und schon wird der "buff" automatisch gelöscht und du sparst dir das 
Post-It.
sizeof(unsigned char) ist übrigens immer 1.

[Daniel A. (daniel-a)]

Danke für die ganzen Hinweise.

Ich werde diese natürlich berücksichtigen, sobald ich Zeit habe.

[g457 (Gast)]

> Das ist kein leak, weil:

Doch, ist ein klassischer Leak.

> a) c_worker::buff nur einmal, bei der Initialisierung speicher reserviert

..jede Instanz von c_worker

> b) Immer benötigt wird

..maximal so lange wie die jeweilige Instanz von c_worker lebt.

> c) Das Programm nie beendet wird

Sowas gibts per se nicht.

Glaubs mir. Räum auf, sonst findest Du die anderen Leaks nie.

[hmm... (Gast)]

Lass mal clang mit dem Analyzer drüber laufen und beseitige ALLE 
Warnungen die geworfen werden. Dann reden wir weiter...

[Johann L. (gjlayde)]

Core dump erstellen lassen und das post-mortem Dump analysieren, z.B. 
mit Debugger.

Je nachdem, was für ein Fehler es ist, kann es kontraproduktiv sein, auf 
Verdacht die Quelle an X Stellen zu ändern.  Wenn die Fehlstelle nicht 
dabei ist (wahrscheinlich) und die Änderungen den Bug lediglich 
maskieren, grüßt spätestens in ein paar Wochen das Murmeltier...

[mar IO (Gast)]

Schau dir mal POCO http://pocoproject.org an! Hier findest Du schon 
einige Bauteile, die Du gleich verwenden kannst.

WebSocket findest Du in der Doku unter Poco::Net WebSocket.