Czy da się wyśledzić: Exception thrown: read access violation

pytanie zadane 18 kwietnia 2018 w C i C++ przez Łukasz Wasilewski Mądrala (5,190 p.)

Cześć forumowicze.

Mam pewien olbrzymi problem przy czyszczeniu pamięci.

Wywala mi błąd Exception thrown: read access violation, którego mimo, że wiem co oznacza nie potrafię wyśledzić.
Poniżej wstawiam zdjęcia z błędu, pierwsze pokazuje gdzie program się zatrzymuje, drugi "Call Stack", natomiast trzeci przedstawia dekonstruktor obiektu klasy którą chcę usunąć. Dekonstruktor usuwa level, z którego pochodzi screen nr. 1 (obie pętle wykonują się poprawnie, błąd wywala po nich), do p_1 (player) nawet nie dochodzi.

Nawet jeśli nie chce Wam się śledzić działania programu i czytać kod, proszę chociaż o jakiekolwiek wskazówki jak mogę namierzyć gdzie problem występuje.

Bardzo proszę o pomoc, gdyż zjadłem na tym już sporo nerwów dzisiaj i nie umiem sobie z tym poradzić.
Link do repozytorium tutaj

Problem pojawia się w klasie game(.cpp) podczas usuwania obiektu klasy map_level

// Quae nocent, docent

3 odpowiedzi

odpowiedź 19 kwietnia 2018 przez mokrowski Mędrzec (155,460 p.)
wybrane 24 kwietnia 2018 przez Łukasz Wasilewski

Najlepsza

W wielu miejscach pomaga nawet trzymanie referencji na obiekt a nie wskaźnika. Jeśli nie będziesz chciał przejmować go na własność, nie będziesz brał odpowiedzialności za zniszczenie. W przypadku obiektów które masz zniszczyć przy wyjściu z cyklu życia obiektu, stosuj std::unique_ptr albo nawet const std::unique_ptr. Jeśli przekazujesz go na zewnątrz to... po pierwsze się zastanów czy robisz to świadomie ew. const (bo to ogólnie ryzykowna praktyka) a jeśli już wiesz że chcesz, przekaż std::shared_ptr. unique nie poddaje się kopiowaniu ale jest "lekki", shared poddaje się kopiowaniu ale jest cięższy bo zlicza referencje. Jeśli zasób może być zniszczony przed jego użyciem, std::weak_ptr (np. callbacki, cache itp).

Doczytaj bo w tym kodzie pakujesz się w tarapaty.

A co do samego błędu.. debug nie działa? :)

komentarz 19 kwietnia 2018 przez Łukasz Wasilewski Mądrala (5,190 p.)

Problem w tym, że vectory te muszą przechowywać wskaźniki, ponieważ obiekty zawarte w nich są tworzone dynamiczne poprzez czytanie mapy z pliku.

Wektory są prywatne, natomiast elementy wyciągam za pomocą getterów, który zwraca nullptr jeśli jest to koniec danego wektora.

Podczas działania programu nie wywala błędu lecz dopiero podczas jego czyszczenia.

Debug zatrzymuje się w miejscu które podałem, następnie wchodzi w miejsca których już nie do końca rozumiem.

komentarz 19 kwietnia 2018 przez mokrowski Mędrzec (155,460 p.)

Problem w tym, że vectory te muszą przechowywać wskaźniki, ponieważ obiekty zawarte w nich są tworzone dynamiczne poprzez czytanie mapy z pliku.

Ale ok, trzymaj. Nic nie zmieni z punktu widzenia semantyki wywołania jeśli to nie będą gołe wskaźniki a std::unique_ptr. Także wyłuskasz przez gwiazdkę, strzałka -> działa i dodatkowo nie zapomnisz czyścić tego wektora bo niszcząc unique, zniszczy elementy.

Wektory są prywatne, natomiast elementy wyciągam za pomocą getterów, który zwraca nullptr jeśli jest to koniec danego wektora.

I znowu.... unique_ptr jeśli nie zawiera nic, ma operator bool który zwraca false. Sensowniej to wygląda niż ... "zakamuflowany std::optional".

W swoim kodzie nie stosujesz const-correctness...

Przeglądam repo...

komentarz 19 kwietnia 2018 przez Łukasz Wasilewski Mądrala (5,190 p.)

Kurcze, zdziwiliście mnie z tymi wskaźnikami, ale brzmi sensownie. Będę działał, dzięki serdecznie. :)

komentarz 19 kwietnia 2018 przez Łukasz Wasilewski Mądrala (5,190 p.)

Po zmienieniu wszystkiego na unique_ptr podczas kompilacji wystąpił taki problem:

Screen

Wiesz może z czego on się bierze?

komentarz 19 kwietnia 2018 przez mokrowski Mędrzec (155,460 p.)

I Bingo! Pokazał Ci paluchem co robisz żle :) W jakimś miejscu robiłeś kopię accesories. Komunikat oczywiście mówi że nie można wywołać konstruktora kopiującego z argumentem std::unique_ptr<accesories>.

Radził bym Ci napisać trochę prostych testów z inteligentnymi wskaźnikami. Zrozumiesz wtedy semantykę.

komentarz 1 maja 2018 przez Łukasz Wasilewski Mądrala (5,190 p.)

@mokrowski, ostatecznie okazało się, że problem leżał gdzie indziej..

Moją ostatnią aktualizacją było wprowadzenie zapisu oraz odczytu pliku mapy jako pliku binarnego.

Problem stanowiła nazwa mapy ukryta w pliku.

Zapisywałem ją tak:

string mapName; // zmienna prywatna klasy

file.write((char *)mapName.c_str(), strlen(mapName.c_str()));

Natomiast odczytywałem:

mapName.resize(mapHeader.numOfTitleChars);
file.read((char *)&mapName, mapHeader.numOfTitleChars);

Gdy robiłem w ten sposób wszystko działało, nazwa wyświetlała się poprawnie. Problem pojawiał się gdy usuwałem obiekt klasy map_level który posiadał właśnie mapName.

Podejrzewam, że chodzi o problem ze zwalnianiem pamięci, ale nie rozumiem dlaczego wywalało Read access violation dopiero przy usuwaniu tego obiektu.

odpowiedź 18 kwietnia 2018 przez Mateusz Tocha Bywalec (2,560 p.)

Co zawiera vector<*accesories> ? widzę że sama klasa accesories jest to jakąś abstrakcją, kto po niej dziedziczy? I jak nadpisywany jest destruktor dla tej klasy.

Pozdrawiam

Na szybko próbuje sobie uruchomić twoje repo, zaraz będe wiedział wiecej.

komentarz 18 kwietnia 2018 przez Łukasz Wasilewski Mądrala (5,190 p.)

Cześć, uruchomić raczej nie będziesz mógł, ponieważ na repo nie wrzucam grafiki, która jest potrzebna do uruchomienia.

Jeśli chodzi o dziedziczenie, to dziedziczą po niej poniższe klasy, oraz klasa enemies. Konstruktora nie nadpisuje, ponieważ żadna z tych klas tego nie wymaga (nie alokuje pamięci).

komentarz 18 kwietnia 2018 przez Mateusz Tocha Bywalec (2,560 p.)

Złożone to, rozważałeś użycie smart pointerów ?

komentarz 18 kwietnia 2018 przez Łukasz Wasilewski Mądrala (5,190 p.)

Szczerze mówiąc pierwszy raz o nich słyszę. Zabieram się za naukę, dam znać czy pomogło.

komentarz 18 kwietnia 2018 przez Mateusz Tocha Bywalec (2,560 p.)

shared_ptr to taki ptr , który może być współdzielony przez wiele różnych obiektów, ma sobie mechanizm ControlBlock który mówi mu jak wiele obiektów jeszcze go używa, a co najważniejsze dla Ciebie , czyści po sobie. Wejdź sobie na youtuba wpisz Cherno SmartPointers (niestety mam teraz internet satelitarny nie moge wysłać Tobie linku)

odpowiedź 19 kwietnia 2018 przez criss Mędrzec (172,590 p.)

Moim zdaniem w którymś miejscu źle/nieświadomie korzystasz z copy/move constructora map_level. Kopiujesz gdzieś obiekt map_level? Masz poprawnie zdefiniowane copy i move constructory i assignment operators? Skoro w tych vectorach trzymasz raw pointery, to powinieneś mieć.

Popatrz: Załóżmy ze masz obiekt map_level z niepustymi vectorami wskaźników na dynamic allocated obiekty. Jeśli go skopiujesz domyślnym copy constructorem czy assignment operatorem to dostajesz drugi obiekt map_level z vectorem ze wskaźnikami wskazującym na te same obiekty. Jeśli teraz usuniesz oryginalny obiekt (chociażby wyjdzie poza swój scope), to wskaźniki w kopii staną się invalid. Jeśli spróbujesz z nich skorzystać to dostaniesz właśnie błąd tego typu jak pokazałeś. Jeśli uruchomii się destruktor na obiekcie-kopii - to samo - próba zwolnienia niezaalokowanej pamięci. To samo by się stało w przypadku gdybyś skopiowal domyślnym move constructorem / assignment op, ale o tym nie wspominam bo jako ze masz user-defined destruktor, to move-wariantów zwyczajnie domyślnie nie ma.

komentarz 19 kwietnia 2018 przez Łukasz Wasilewski Mądrala (5,190 p.)

Dziękuję za pomoc, bardzo fajnie mi to wyjaśniłeś.

Jeśli chodzi o problem natomiast, to obiektu mapy nie kopiuję. Jest ona jedna, wczytywana z pliku i wszystkie obiekty które tworzy podczas wczytywania powinny być razem z nią usuwane.

Wskaźników które przechowuje w wektorze używam w innej klasie, natomiast z tym nie mam problemu. Właśnie będę kombinował z unique_ptr tak jak mi podpowiedzieli koledzy u góry. :)

1	komentarz 19 kwietnia 2018 przez criss Mędrzec (172,590 p.) Hm w takim razie pewnie trzeba zobaczyć więcej kodu.. :/ Anyway wspomniane wyżej metody zdefiniuj albo zabron ich tworzenia (=delete) jeśli nie potrzebujesz nigdy kopiować mapy.