Programm stürzt zufällig ab

Bengo

Folgendes Programm:

int main(int argc, char** argv) {

    for (int i = 4; i < 1000; ++i) {
        Faktor* fac = new Faktor(i);
        int lastNumber = 0;
        while(1) {
            int number = fac->getNextNumber();
            if (number == lastNumber) {
                cout << i << ": Ist prim" << endl;
                lastNumber = number;
                delete fac;
                fac = new Faktor(number);
                break;
            }
            lastNumber = number;
            if (number > 100000) {
                cout << i << ": Zu groß zum Berechnen" << endl;
                delete fac;
                fac = new Faktor(number);
                break;
            }
            delete fac;
            fac = new Faktor(number);
        }
        delete fac;
    }

}

sürzt mir nach einer relativ zufälligen Anzhal von Schleifendurchgängen der for Schleife ab. Ich bekomme dann sowas wie 17: Ist pri und sonst gibt es nichts. Mit dem Debugger habe ich herausgefunden, dass es auch kein Speicherleck gibt, da die Adresse von fac, immer die selbe bleibt.
Mir ist bewusst, dass es eine Endlosschleife geben kann, das ist auch gewollt, konnte aber in meinen Testdurchläufen nicht auftreten.
Faktor ist im Prinzip nur eine Klasse, die einen std::vector verwaltet.

Hyde++ 0

Das sieht merkwürdig aus. Wieso denn immer diese new-delete-Paare? Nimm doch ein Objekt auf dem Stack und implementiere eine clear()-Memberfunktion oder so.

Bengo

Was mich noch etwas mehr verwirrt, ist das das Programm später abstürzt, je größer ich die for Schleife ansetzen, also wenn ich 1000 durch 10000 ersetze schafft das Programm plötzlich 4000 statt 200, wie sonst.

Bengo

ok das hat mich auf die entscheidende Idee gebraucht, nur die Ausgabe meiner IDE (NetBeans) funktionert nicht. Auf der Konsole läuft es einwandfrei.

Mechanics

Ich glaube nicht, dass du die entscheidende Idee schon hattest. new und delete sind ganz schlechte Ideen. Ich hab kein Lust mich da reinzudenken, aber ich vermute, dass die eben nicht zusammenpassen. Sowas führt dann eben zu Abstürzen oder zu Memory Leaks und ist auf jeden Fall langsam. Dafür gibts keinen Grund. In C++ verwendet man bevorzugs automatische Objekte oder notfalls Smartpointer, aber nicht sowas.

Bengo

Da hast du sicherlich recht.
Was genau sind automatische Objekte? (Also das ganze mit Konstruktor Desstruktor?)
Smartpointer klingt auch nicht schlecht, warum nur notfalls?

SeppJ

Bengo schrieb:

Was genau sind automatische Objekte? (Also das ganze mit Konstruktor Desstruktor?)

Das was du wahrscheinlich als "normale" Variablen bezeichnen würdest. Also einfach Objekte, die in einem Codeblock definiert werden:

int main()
{
  int i; // automatisches Objekt

  {
     double d;   // Auch automatisches Objekt, in einem tieferen Gültigkeitsbereich
  }  // Ab hier ist d ungültig

}  // Ab hier ist i ungültig

Diese sind gültig von ihrer Definition an bis zum Ende des Codeblocks, in dem sie stehen (also die schließenden geschweiften Klammern). Sie werden automatisch erzeugt und zerstört, daher der Name. Das bedeutet bei Klassenobjekten gegebenenfalls auch einen automatischen Aufruf des Konstruktors bzw. Destruktors. Und das hat ziemlich nützliche Konsequenzen, denn egal wie kompliziert oder unvorhersehbar der Programmfluss wird (Exceptions!), es ist garantiert, dass der Konstruktor und vor allem der Destruktor an der richtigen Stelle aufgerufen wird. Wenn das Objekt beispielsweise irgendeine Art von Ressource verwaltet, die vom Destruktor freigegeben wird, braucht man sich um gar nichts zu kümmern; die Ressource wird ganz sicher an der richtigen Stelle freigegeben (und auch garantiert nur genau einmal, also keine Möglichkeit von Fehlern wie doppelten Freigaben). Mit expliziten news und deletes im Code ist das hingegen praktisch unmöglich richtig hin zu bekommen.

Smartpointer klingt auch nicht schlecht,

Meinst du damit den Klang von "Smart" oder hast du wirklich mal geguckt, was das ist und wo man sie benutzt?

warum nur notfalls?

Weil die dynamische Speicherverwaltung nur Nachteile hat, so lange man nicht eine ihrer besonderen Eigenschaften benötigt.

Bengo

Ok danke.
Dass Variablen schon nach Verlassen des Blockes ungültig werden, hatte ich gerade völlig übersehen, dann macht das ganze auch Sinn
Das Konzept der Smartpointer kenne ich, und finde es auf jeden Fall sicherer als das normaler Pointer.

Was sind denn diese besonderen Eigenschaften?

Ist new und delete eigentlich ein wirklich absolutes nogo, oder nur eine sehr gute Richtlinie?

Bei meinem Programm ist ersteres etwas schwierig umzusetzen, ich müsste in einem Block den Destructor und dann den Konstructor aufrufen. Bin mir nicht sicher, wie das gehn soll.

SeppJ

Bengo schrieb:

Was sind denn diese besonderen Eigenschaften?

Na, dass sie dynamisch ist. Das heißt Größe und Lebenszeit stehen nicht unbedingt fest. Wobei einen meistens nur die Größe interessiert und die Lebenszeit immer noch der lokale Gültigkeitsbereich ist.

Ist new und delete eigentlich ein wirklich absolutes nogo, oder nur eine sehr gute Richtlinie?

Wenn du Smartpointer und andere Container kennst, welchen Grund könntest du jemals haben, noch new und delete zu nutzen? Wenn dir ein Grund einfällt, dann hast du höchstwahrscheinlich die Smartpointer/Container nicht richtig verstanden.

Mechanics

Bengo schrieb:

Ist new und delete eigentlich ein wirklich absolutes nogo, oder nur eine sehr gute Richtlinie?

Das ist eine philosophische Frage. Ich würde es nicht unbedingt als no go bezeichnen, vor allem, wenn man weiß, was man tut. Aber es ist meist die schlechteste Alternative.

Du brauchst dynamische Speicherverwaltung z.B., wenn du Objekte weitergeben willst und die auch nach Verlassen des Scopes gültig sein sollen. Ist jetzt hier nicht der Fall.

Mechanics schrieb:

Das ist eine philosophische Frage. Ich würde es nicht unbedingt als no go bezeichnen, vor allem, wenn man weiß, was man tut. Aber es ist meist die schlechteste Alternative.

Du brauchst dynamische Speicherverwaltung z.B., wenn du Objekte weitergeben willst und die auch nach Verlassen des Scopes gültig sein sollen. Ist jetzt hier nicht der Fall.

Rohe Pointer sind ein Nogo! Dein "Anwendungsfall" ist ein Fall für unique_ptr+release(). Denn du willst deine Objekte wahrscheinlich zerstören, wenn eine Exception geworfen wird und nur freigeben, wenn nicht.

Bengo

Würde ich mein Problem nun am besten mit einem Smartpointer lösen, oder besser der Klasse eine methode zum überschreiben mitgeben?

Ist der Speicher im Stack nicht begrenzt und soll man größere Daten nicht auf den Heap ablegen. Dafür dann die Container der std, weil es sowieso erst bei vielen Objecten ins Gewicht fällt?

Bengo schrieb:

Was sind denn diese besonderen Eigenschaften?

Ich weiß nicht ob SeppJ das damit meinte aber eine Eigenschaft von dynamische Speicherverwaltung ist natürlich, dass diese dynamisch ist! Wenn ich also erst zur Laufzeit weiß, wie viel Speicher ich für irgendwelche Daten brauche kommt man um dynamischen Speicher nicht drumherum. Allerdings helfen die Container aus der STL das man es nicht selbst machen muss.

Bengo schrieb:

Ist new und delete eigentlich ein wirklich absolutes nogo, oder nur eine sehr gute Richtlinie?

Wenn man es überall verbieten würde, dann könnte man ja nicht sowas wie std::vector implementieren. Allerdings wurde das einem schon von den Entwicklern der Standardlibrary abgenommen, sodass man selbst eigentlich kaum noch new/delete schreiben braucht. Wenn man unbedingt in seiner Klasse Speicherverwaltung selbst machen möchte kann man ein new im Konstruktor und ein delete im Destruktor nutzen. Zwei Pointer in einer Klasse sind bereits nicht mehr trivial!

unique_philosoph schrieb:

Rohe Pointer sind ein Nogo! Dein "Anwendungsfall" ist ein Fall für unique_ptr+release(). Denn du willst deine Objekte wahrscheinlich zerstören, wenn eine Exception geworfen wird und nur freigeben, wenn nicht.

Warum unique_ptr wenn man es auch auf dem Stack machen kann? Außerdem was soll der unterschied zwischen zerstören und freigeben sein? Man will natürlich, dass Objekte immer freigegeben werden!

Bengo schrieb:

Würde ich mein Problem nun am besten mit einem Smartpointer lösen, oder besser der Klasse eine methode zum überschreiben mitgeben?

Die einfachste Variante für dein Programm sähe so aus:

for (int i = 4; i < 1000; ++i) {
    Faktor fac(i);
    // [...]
}

Hier ist allerdings der Nachteil, dass in jedem Schleifendurchlauf immer ein neues Objekt angelegt wird. Je nachdem wie groß dein Faktor Objekt ist könnte sowas schneller sein:

Faktor fac;
for (int i = 4; i < 1000; ++i) {
    fac.set(i);
    // [...]
}

Smart Pointer brauchst du hier nicht.

sebi707 schrieb:

unique_philosoph schrieb:

Rohe Pointer sind ein Nogo! Dein "Anwendungsfall" ist ein Fall für unique_ptr+release(). Denn du willst deine Objekte wahrscheinlich zerstören, wenn eine Exception geworfen wird und nur freigeben, wenn nicht.

Warum unique_ptr wenn man es auch auf dem Stack machen kann? Außerdem was soll der unterschied zwischen zerstören und freigeben sein? Man will natürlich, dass Objekte immer freigegeben werden!

Unglückliche Wortwahl, mit freigeben habe ich "nicht zerstören" gemeint. Und ich bezog mich auf den Anwendungsfall von Mechanics, der das aus irgendeinem Grund vorausgesetzt hat.

sebi707 schrieb:

Wenn man es überall verbieten würde, dann könnte man ja nicht sowas wie std::vector implementieren.

Wieso, std::vector ist ein perfekter Nutzungszweck für unique_ptr!

struct vector {
  unique_ptr<aligned_storage<sizeof(T), alignof(T)>[]> data;
  size_t len;
  size_t alloc;

  void resize(int n) {
    unique_ptr<aligned_storage<sizeof(T), alignof(T)> new_data(new aligned_storage<sizeof(T), alignof(T)>[n]);
    uninitialized_copy(...); // daten rüberkopieren
    data = move(new_data);
    alloc = n;
  }
};

Bengo

sebi707 schrieb:

Die einfachste Variante für dein Programm sähe so aus:
for (int i = 4; i < 1000; ++i) {
    Faktor fac(i);
    // [...]
}
Hier ist allerdings der Nachteil, dass in jedem Schleifendurchlauf immer ein neues Objekt angelegt wird. Je nachdem wie groß dein Faktor Objekt ist könnte sowas schneller sein:
Faktor fac;
for (int i = 4; i < 1000; ++i) {
    fac.set(i);
    // [...]
}
Smart Pointer brauchst du hier nicht.

Problem ist aber, dass ich dann in meiner While Schleife erneut den Konstruktor aufrufen müsste, was ich nicht kann. Ich denke das sinnvollste ist wirklich diese Lösung und dann noch eine init_neu(int number) Methode.

Bengo schrieb:

Problem ist aber, dass ich dann in meiner While Schleife erneut den Konstruktor aufrufen müsste, was ich nicht kann.

Aber einen Zuweisungsoperator hast du doch?

fac = Faktor(number);

sollte automatisch gehen.

Mechanics

unique_philosoph schrieb:

Unglückliche Wortwahl, mit freigeben habe ich "nicht zerstören" gemeint. Und ich bezog mich auf den Anwendungsfall von Mechanics, der das aus irgendeinem Grund vorausgesetzt hat.

Ich hab gar nichts vorausgesetzt, ich weiß nicht, was du da reininterpretierst. Und ich hab nichts über rohe Zeiger gesagt.

unique_philosoph schrieb:

Wieso, std::vector ist ein perfekter Nutzungszweck für unique_ptr![cpp]

In deinem Beispiel steht aber noch ein new . Du hättest make_unique benutzen sollen! Aber irgendwo muss am Ende aber auf jeden Fall ein new stehen müssen.

Was soll eigentlich das ganze aligned_storage Klimbim? Wenn ich new T[N] schreibe ist das ja wohl schon passend aligned für T.

Bengo

unique_philosoph schrieb:

Bengo schrieb:

Problem ist aber, dass ich dann in meiner While Schleife erneut den Konstruktor aufrufen müsste, was ich nicht kann.

Aber einen Zuweisungsoperator hast du doch?
fac = Faktor(number);
sollte automatisch gehen.

Also mein Programm würde dann so aussehen:

int lastNumber = 0;
    for (int i = 20; i < 91000; ++i) {
        Faktor fac(i);
        int counter = 0;
        while(1) {
            int number = fac.getNextNumber();
            counter ++;
            fac = Faktor(number);
            if (number == lastNumber) {
                cout << i << ": Geht auf prim " <<counter << endl;
                lastNumber = number;
                break;
            }
            lastNumber = number;
            if (number > 100000) {
                cout << i << ": Zu groß zum Berechnen "<<counter << endl;           
                break;
            }  
            counter ++;
        }
    }

Den zuweisungsoperator kann ich doch dann so definieren und implementieren lassen?:

Faktor& operator=(const Faktor&& rhs)=default;

unique_philosoph schrieb:

Bengo schrieb:

Problem ist aber, dass ich dann in meiner While Schleife erneut den Konstruktor aufrufen müsste, was ich nicht kann.

Aber einen Zuweisungsoperator hast du doch?
fac = Faktor(number);
sollte automatisch gehen.

Super! Statt jeden Schleifendurchlauf ein neues Objekt zu erstellen, erstellen wir immer noch jeden Schleifendurchlauf ein Objekt und kopieren dieses noch zusätzlich!