[C++] Problem z wskaźnikami współdzielonymi. Nieznana przyczyna wycieku pamięci.

[Wonski]

Cześć, witam wszystkich
Ostatnio postanowiłem poszerzyć swoją wiedzę o smart pointery z biblioteki stl-a.
Napisałem prostą hierarchię dwóch klas (w dużym uproszczeniu można by nazwać ją drzewem)
Dla przykładu:

Kod: [Zaznacz]

class Key
{

shared_ptr< string > _key_name;
shared_ptr< string > _key_value;

public:
shared_ptr<string> get_key_name()
{
return this->_key_name;
};

void set_key_name(shared_ptr<string> _helper)
{
this->_key_name = _helper;
};

shared_ptr<string> get_key_value()
{
return this->_key_value;
};

void set_key_value(shared_ptr<string> _helper)
{
this->_key_value = _helper;
};
};
oraz zawierająca klasę Key, klasa Section:

Kod: [Zaznacz]

class Section
{
vector< shared_ptr< Key > > _key_vector;

public:

shared_ptr< Key > get_key(shared_ptr<string> _helper)
{
size_t i;
for (i = 0; i < _key_vector.size();)
{
if (this->_key_vector.at(i)->get_key_name()->c_str() == _helper->c_str())
return this->_key_vector.at(i);

else
i++;
}
return nullptr;
}

void add_key(shared_ptr<string> _name_help, shared_ptr<string> _value_help)
{
auto _key_help = make_shared<Key>();
_key_help->set_key_name(_name_help);
_key_help->set_key_value(_value_help);

this->_key_vector.push_back(_key_help);
}
};
Problem jest następujący...
Gdy próbuję wywołać ten kod:

Kod: [Zaznacz]

shared_ptr<Section> _section = make_shared<Section>();
shared_ptr<Key> _key = make_shared<Key>();
{
auto _key_name = make_shared<string>("key_name");
auto _key_value = make_shared<string>("key_value");
auto _section_name = make_shared<string>("section_name");

_key->set_key_name(_key_name);
_key->set_key_value(_key_value);

_section->add_key(_key_name, _key_value);
}

auto _k_name = make_shared<string>("key_name");

if (_section->get_key(_k_name) == nullptr)
cout << "sdasdasd" << endl;
już w samej głównej funkcji main, oczywiście, to warunek if zwraca prawdę, a nie powinien.
Operacje na obiektach specjalnie dałem w bloku bo chciałem sprawdzić czy kontrola rzeczywiście zmniejsza licznik referencji po opuszczeniu scope'a no i wgl chciałem "zasymulować" funkcję. (wszystko na potrzeby uproszczenia przedstawienia problemu)

Wszystkie operacje na obiektach implementuję poprawie. (choć mam wątpliwości czy w setterach nie powinienem wartości wskaźnika przypisywać za pomocą funkcji move()).
Debbuger wywala błąd access volation.. i tu jestem w ślepej uliczce, ponieważ nawet gdy wykonuję program krok po kroku to nie mogę wychwycić źródła tego problemu.

Dodam, że ta struktura jest bardzo uproszczona na potrzeby jak najprostszego przedstawienia problemu. Jeżeli ktoś ma ochotę zapoznać się z całym projektem (zaledwie kilka klas) to zapraszam na mojego gita:
https://github.com/SztywnyPawlo/INI_Parser
Kod znajdujący się w tym wątku to uproszczony kod właśnie tego projektu.
Tam mimo że struktura jest bardziej zagnieżdżona występuje ten sam problem.

Oczywiście zdaję sobie sprawę, że tą strukturę moznaby napisać znacznie łatwiej bez użycia smart pointerów, ale tu chodzi właśnie o edukacje
Pozdrawiam i czekam na odpowiedź.

[inż. Avallach]

Sprawdź gdzie wywala to access violation... jaki plik, jaka linijka, przy próbie odczytu czego. Jakbym miał zgadywać, próbujesz czytać niezainicjalizowany wskaźnik (odpowiednik javowego NullPointerException).

[Wonski]

wywala w pliku memory.h
linia 349:

Kod: [Zaznacz]

_Reset(_Other._Ptr, _Other._Rep);
jest to ciało metody:

Kod: [Zaznacz]

void _Reset(const _Ptr_base<_Ty2>& _Other)

tak btw:
Ile zajęła Ci nauka c++?
Wiem, ze tego można się uczyć w nieskończoność... bardziej mi chodzi o ilość nauki by można zacząć pracę w tym sektorze.

[inż. Avallach]

Jeden semestr, ale miałem lata doświadczenia w C# i Javie, a do tego kompetentnego i wymagającego wykładowcę i prowadzącego laboratoria w jednej osobie.

Jeśli wywala ci w tej linii, to oczywiste jest że _Other jest niezainicjalizowane. Możesz potwierdzić to wstawiając tam jakiegoś rodzaju asercję na _Other != nullptr. Swoją drogą bardzo polecam stosowanie się do jakiejś rozsądnej konwencji nazewniczej - to co obecnie masz to jest jakiś kiepski żart wyraźnie utrudniający rozumienie kodu.

[Wonski]

No nazwy zmiennych, metod mają być opisowe i zrozumiałe. A poza tym jak czytałeś (chyba) jest to tylko okrojony przykład, szkic, prototyp, więc niewykluczone, że nazewnictwo może być dla profesjonalisty śmieszne..
No i jakbyś mógł to podaj konkret, bo ciężko jest się domyślić o o co chodzi jeżeli nie ma się o tym pojęcia, rozumiesz nie?

[inż. Avallach]

Pierwszy przykład z brzegu: "_Reset(_Other".
Zarówno metodę, jak i zmienną (pole?) piszesz prefixując podkreślnikiem i używając upper camel case. Raz że nie da się ich przez to odróżnić i trzeba zgadywać co jest czym (bo _Reset mogłoby być np także zmienną typu który ma przeładowany operator wywołania i ja tylko zgaduję że to metoda), dwa że tej konkretnej konwencji nie stosuje chyba nikt na świecie wobec żadnego rodzaju bytu.

Według konwencji do których ja jestem przyzwyczajony, nazwy klas pisze się upper camel casem ("class Section"), nazwy funkcji, metod i zmiennych lokalnych lower camel casem ("getKey(helper)"), a nazwy pól lower camel casem prefixowanym przez podkreślnik ("_keyVector").
Nie musisz stosować się do mojej. Stosuj się do jakiejkolwiek która jest rozsądnie często używana i którą będziesz stosował konsekwentnie. Kilka przykładów:
https://google.github.io/styleguide/cppguide.html#General_Naming_Rules
https://msdn.microsoft.com/en-us/library/ms229043.aspx

[Wonski]

Pierwszy przykład z brzegu: "_Reset(_Other".
Zarówno metodę, jak i zmienną (pole?) piszesz prefixując podkreślnikiem i używając upper camel case. Raz że nie da się ich przez to odróżnić i trzeba zgadywać co jest czym (bo _Reset mogłoby być np także zmienną typu który ma przeładowany operator wywołania i ja tylko zgaduję że to metoda), dwa że tej konkretnej konwencji nie stosuje chyba nikt na świecie wobec żadnego rodzaju bytu.

Nie rozumiem?
Pytałeś się gdzie konkretnie wywala błąd to powiedziałem, ze w pliku memory.h.
Jako programista c++ powinieneś wiedzieć, że jest to plik nagłówkowy stl-a, który zawiera definicje klas inteligentnych wskaźników no i jak sama nazwa wskazuje: resztę ogólnych mechanizmów zarządzania pamięcią.
Czyli metoda _Reset nie jest moja
Tak jako ciekawostkę powiem, że większość elementów biblioteki standardowej pochodzi chyba z boosta, w tym i właśnie smart pointery.
No i nie wiem czy tej konwencji NIKT nie stosuje, bo jakbyś zajrzał do implementacji biblioteki standardowej to cały kod jest tak napisany, a piszą to twórcy tego języka. (a do tego zamiast stosować consty to nadal używają makr, co w modern c++ jest dosyć dziwne i powiedziałbym: niedorzeczne. Sam odchodzę od makr na rzecz wyżej wspomnianych constów i funkcji szablonowych)
Owszem nie sposób się nie zgodzić z tym, że jest to nieczytelne, no ale to nie moja wina

A w moim kodzie (ten który jest przedstawiony w temacie) jak sam możesz zauważyć, nie mam takiego pierdolnika. Tzn nie zaczynam nazw metod i pól od "_", stosuję to tylko do pól... samą konwencje podpatrzyłem gdzieś na jakimś forum, gdzie jakiś typek robił frameworka pod DX11.
Stamtąd też wziąłem pomysł, by w definicji klasy na początku dawać konstruktor i destruktor, potem atrybuty, następnie metody publiczne a na sam koniec metody prywatne.
Wg mnie to jest bardzo dobra i czytelna konwencja.

Wgl konwencje pisania kodu to wg mnie bardzo delikatna sprawa.

#edit
No i wgl nwm jak można by zastosować Twoją konwencję do funktorów. No bo klasa po przeciążeniu operatora () to jakby i obiekt i funkcja jednocześnie
Czyli gdyby za agregować taki obiekt jako element innej klasy, to co wtedy byśmy uzyskali? Atrybut czy metodę?

[inż. Avallach]

Olej moje dwa ostatnie posty, mała awaria mózgu xD
Implementacje biblioteki standardowej są bardzo różne i mają czasami dziwne konwencje. Fakt że się trochę skompromitowałem biorąc fragment jako przykład *twojego* złego nazewnictwa.

większość elementów biblioteki standardowej pochodzi chyba z boosta

To nie tak. Implementacje mogą być różne. Wiele elementów (jak właśnie smart pointery) ma design wzorowany  na starszych odpowiednikach zaimplementowanych najpierw w boost. Ale kod jest pisany według konwencji specyficznych dla danej implementacji biblioteki standardowej.

jakbyś zajrzał do implementacji biblioteki standardowej to cały kod jest tak napisany, a piszą to twórcy tego języka

Twórcy języka (a właściwie: komitet standaryzacyjny WG21) określają tylko interfejs i zachowanie. Implementacje są zazwyczaj dostarczane przez twórców kompilatorów.

nie zaczynam nazw metod i pól od "_", stosuję to tylko do pól...

To jest element także konwencji którą ja stosuję. Niestety nie trzymasz się jej poprawnie - nazywasz tak także wszystkie zmienne lokalne i parametry które pokazałeś.
Pole to specjalna zmienna - taka która jest członkiem klasy.

nwm jak można by zastosować Twoją konwencję do funktorów. No bo klasa po przeciążeniu operatora () to jakby i obiekt i funkcja jednocześnie
Czyli gdyby za agregować taki obiekt jako element innej klasy, to co wtedy byśmy uzyskali? Atrybut czy metodę?

Funktor to obiekt. Jeśli jest trzymany jako member, to według mojej konwencji prefixujesz jego nazwę podkreślnikiem. Metoda to wyraźnie co innego - ma dostęp do pól klasy i deklaruje się ją za pomocą innej składni niż funktor.

[Wonski]

Twórcy języka określają tylko interfejs i zachowanie. Implementacje są zazwyczaj dostarczane przez twórców kompilatorów.

No a jak to ma się bo modułów typu experimental?
Szczerze to myślałem, że STL jest jeden i uniwersalny, opracowany właśnie przez gości z komitetu.

To jest element także konwencji którą ja stosuję. Niestety nie trzymasz się jej poprawnie - nazywasz tak także zmienne lokalne i parametry.
Pole to zmienna będąca członkiem klasy.

No to wiem. Stosowałem to raczej by odróżnić metody od zmiennych, bo zmienne lokalne odróżnia już samo IDE, tzn. zmienne lokalne są oznaczone na szaro (piszę w visual studio) a atrybuty to normalny biały tekst.

No to skoro uzgodniliśmy sprawę konwencji i skoro mam olać Twoje dwa pierwsze posty to problem zawarty w temacie nadal pozostaje nierozwiązany
Nie zależy mi na szybkości odpowiedzi, spokojnie XD

[inż. Avallach]

Wklej cały stack trace.

[Wonski]

Proszę:

Kod: [Zaznacz]

> Project2.exe!std::_Ptr_base<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >::_Reset<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >(const std::_Ptr_base<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > & _Other) Line 349 C++
  Project2.exe!std::shared_ptr<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >::shared_ptr<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >(const std::shared_ptr<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > & _Other) Line 495 C++
  Project2.exe!Key::get_key_value() Line 30 C++
  Project2.exe!main() Line 25 C++
  [External Code]
  [Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]


[inż. Avallach]

Pokaż co tu siedzi:
Project2.exe!Key::get_key_value() Line 30   C++

[Wonski]

Kod: [Zaznacz]

shared_ptr<string> get_key_value()
{
return this->_key_value;
};
Wg mnie to zbyt wcześnie zeruje reference_count i dlatego sie sypie.. tzn odwołuje się do już zdealokowanej pamięci. A dlaczego zbyt wcześnie zeruje licznik to właśnie nie mam zielonego pojęcia. Założeniem jest niestety, że te wartości muszą być współdzielone.

[inż. Avallach]

Próbujesz skopiować shared pointera. Jesteś pewien że jest on zainicjalizowany? Że w momencie wołania get_key_value() masz ustawioną jakąś _key_value ?

[Wonski]

No tak. Mimo że, nie ma konstruktora i nie inicjalizuje tego na liście inicjalizacyjnej...
Za umieszczenie tam wartości odpowiada przeciez ten kod:
(już w funkcji main())

Kod: [Zaznacz]

auto _key_name = make_shared<string>("key_name");
auto _key_value = make_shared<string>("key_value");

_key->set_key_name(_key_name);
_key->set_key_value(_key_value);
CHYBA ŻE,....
chodzi Ci o inicjalizacje w konstruktorze klasy Section obiektu klasy Key za pomocą make_shared to nie... ale wydaj mi sie to zbędne, bo jak sam widzisz implementacja którą zastosowałem pozwala na dodanie tylko pełnoprawnych obiektów klasy Key.

[inż. Avallach]

Wrzuć gdzieś aktualne źródła, bo to co pokazujesz nijak nie pasuje do tego co jest w repozytorium które linkowałeś.

Jeśli to co teraz wkleiłeś jest naprawdę w funkcji main(), to chodzi mi o inne _key_value (jedynie tak samo nazwane).
Chodzi o to _key_value, które jest memberem klasy Key - bo to przy próbie jego kopiowania następuje crash.

Kod: [Zaznacz]

// constructor with initialize values
INI_Key(std::shared_ptr< std::string > _key_name_ini_shared_ptr, std::shared_ptr< std::string > _key_value_ini_shared_ptr) :
_key_name_shared_ptr(nullptr), _key_value_shared_ptr(nullptr)
{
this->_key_name_shared_ptr = _key_name_ini_shared_ptr;
this->_key_value_shared_ptr = _key_value_ini_shared_ptr;
};To jest jakaś mała katastrofa co tu wyprawiasz ;p

[Wonski]

No przecież to jest to samo tylko konstruktora nie ma ;d Specjalnie uprościłem by dużo nie czytać i szybko dotrzeć do niezbędnych informacji.
Jak już pisałem to wszystko dla uproszczenia sytuacji
No chyba, ze masz zastrzeżenia do listy inicjalizacyjnej, ale to chciałem zrobić w ten sposób, ze przy próbie odczytu błędnej wartości z Key'a zwróci wartość defaultową, określoną wyżej w hierarchii, ale to jest nieistotne / temat na osobny wątek

Cały kod "sterujący"

Kod: [Zaznacz]

int main()
{

shared_ptr<Section> _section = make_shared<Section>();
shared_ptr<Key> _key = make_shared<Key>();
{
auto _key_name = make_shared<string>("key_name");
auto _key_value = make_shared<string>("key_value");
auto _section_name = make_shared<string>("section_name");

_key->set_key_name(_key_name);
_key->set_key_value(_key_value);

_section->add_key(_key_name, _key_value);
}

auto _k_name = make_shared<string>("key_name");

_section->get_key(_k_name)->get_key_value();

system("PAUSE");
return 0;
}


Widać zmianę w ostatnich linijkach, ale to również jest nieistotne, ponieważ to jest ten sam problem -> odwoływanie się to trefnego wskaźnika.

To jest jakaś mała katastrofa co tu wyprawiasz ;p

No nie wątpię.. ale chciałem by wszystko było jak najprostsze, ale z użyciem smart pointerów.

[inż. Avallach]

I żeby było jak najprościej inicjalizujesz membery dwa razy, najpierw listą inicjalizacyjną na nullptr, a potem przypisaniem na kopię parametru? A potem jeszcze raz setterem?

Kod z użyciem smart pointerów automatycznie jest prostszy i krótszy niż ten ze zwykłymi smart pointerami... Ty za to robisz jakieś niewiarygodne bezsensowne kombinacje jak to co napisałem wyżej ;)

Wrzuć gdzieś cały kod to sprawdzę o co chodzi. Teraz widzę to co jest na Githubie - co w wielu miejscach nie ma sensu i strzępki tego co wrzucasz na forum - co także momentami jest bardzo mocno wątpliwe.

[Wonski]

I żeby było jak najprościej inicjalizujesz membery dwa razy, najpierw listą inicjalizacyjną na nullptr, a potem przypisaniem na kopię parametru? A potem jeszcze raz setterem?

Wiem, że dziwnie to wygląda ale tak ma być
Tzn obsługa wyjątków ma działać jeszcze przed utworzeniem obiektu( lista inicjalizacyjna).

No chyba, że przeoczyłem coś w dokumentacji... i smart pointery nawet jak nie są zainicjalizowane żadną wartością, ot:

Kod: [Zaznacz]

std::shared_ptr< jakis_typ > _obj_smart_ptr;i nawet wtedy wskazują na nullptr to fakt lista inicjalizacyjna jest niepotrzebna. Jako że, w dokumentacji (ani w memory.h) niespecjalnie chciało mi się grzebać a w samej książce, gdzie na smart pointery poświęcono około 50 stron, nic nie ma na ten temat.
Jako że, jestem zapobiegawczy to zrobiłem to w ten sposób

A potem jeszcze raz setterem?

No tak!
Przecież mogę chcieć zmienić wartość klucza w trakcie trwania programu. Ta struktura nie ma służyć tylko pobierania i odczytywania danych z pliku, ale również do jego modyfikacji.

No i ta pętla którą mi tak opisałeś nie ma nigdzie miejsca (w sensie, że wszystkie jej kroki występują kolejno po sobie), wszędzie jest albo sam konstruktor, albo to setter przekazuje wartości do konstruktora.

Kod który wrzuciłem na gita jest wszystkim co do tej pory udało mi się zrobić..
ot znalazłem problem w implementacji, męczyłem się z nim kilka dobrych dni, aż w końcu przyszedłem z tym na forum
No chyba, że życzysz sobie kompleksowe pliki solucji aby u siebie skompilować to również nie ma problemu za chwilę wrzucę całe archiwum na dropboxa i podam link ")

Link do archiwum z solucją:

Kod: [Zaznacz]

https://www.dropbox.com/s/gb3df91b7eahdbj/IniReader.rar?dl=0
tak btw

co w wielu miejscach nie ma sensu

Zawsze gdy pokazuję Ci mój kod to mówisz, że nie ma sensu.. 

[inż. Avallach]

Robisz coś prostego w niesamowicie skomplikowany sposób. A jestem pewien że na żadne z wytłumaczeń które podajesz nie masz napisanych przypadków testowych które te potrzeby potwierdzają. Poczytaj trochę o TDD - to bardzo dobry sposób żeby przestać pisać kod który nie ma sensu, zostawiając tylko te kawałki które sens mają ;p

Przykładowo u mnie w pracy nie można zacommitować do repozytorium kodu który nie jest pokryty testami jednostkowymi... One z jednej strony częściowo uzasadniają potrzebę jego istnienia, a z drugiej pozwalają bezpiecznie dokonywać refactoringów i uproszczeń, nie bojąc się że przypadkiem zmieni się jego zewnętrznie widoczne zachowanie.