Ayyıldız2 | 2008 TR Yapısı • 1-99 Orta Emek Destan • Oto Avsız • 10 Temmuz 21:00 HEMEN TIKLA!
Metin2 source çöplüğünde gözüme takılması üzerine;
C++'da user-declared dtor yazarsanız (kaynak yöneten sınıflarda genelde yazarsınız) derleyici copy ctor ve copy assign operatorunu implicitly declare eder.
`TEMP_BUFFER` bir kaynak yönetiyor (has-a LPBUFFER), destructorunda kaynağı geri veriyor (RAII olarak bilinen, Bjarne tarafından uydurulmuş idiom).
Ama kaynak yöneten bir sınıfınız varsa ve derleyicinin yazdığı copy special member funclarını kullanırsanız senaryoya bağlı olarak başınıza farklı işler gelebilir.
Bİzim durumumuz için varsayalım ki;
gibi bir kod yazdınız; derleyicinin ürettiği copy-assign shallow copy yapar, b1'in eski buffer'ı dtoru görmeden (yani buffer_delete çağrılmadan) üstüne b2.buf yazılır. b1.buf == b2.buf olduğu için copy-ctor'da oluşacak corruption da meydana gelir. Yani assign = leak + corruption.
Copy-ctor'da gördüğüm kadarıyla sadece corruption var. İki nesne aynı `buf`'ı paylaşır, dtorda `buf` iki kez `buffer_delete`'e gider. `buffer_delete` eğer boyut poole sığıyorsa `free` çağırmıyor, buffer'ı free liste geri koyuyor. Aynı `buf` listeye iki kez push edilince mantıken `buf->next = buf` olur, yani next node aslında kendine işaret ediyor. Sonra `buffer_new` aynı `buf`'ı iki ayrı çağrıya dağıtır olası olarak heap overflow, infinite loop tarzı bir şey yaşanabilir. Klasik corruption, alakasız bir yerde patlar diye tahmin ediyorum.
En basit çözüm copy-ctor ve copy-assign operatoru explicit delete etmek amaç önlem almak, zaten aklı başında kimsenin TEMP_BUFFER'ı kopyalayacağını sanmıyorum.
yok ben illa rule of five'a uyucam diyorsanız:
Not: bu deep copy sadece yazılmış veriyi taşır, read_point ve flag kopyalanmaz, yani yarı okunmuş bir buffer'ın tam state'ini korumaz. Paket kurmada yazma için kullanıldığından sorun değil, ama bunu bilerek kullanın.
Not2: dediğim gibi hayati bir şey değil büyük ihtimalla buffer'ı hiç kopyalamamışsınızdır ama benzer diğer kaynak yöneten classlarda da benzer durumları göz önünde bulundurun.
Tek uyarı: user-declared dtor olduğunda move ctor ve move assign operator derleyici tarafından implicitly declate edilmez, yani copy'leri delete edince tip hiç taşınamaz hale gelir. Buffer'ı `std::move`'layan bir yerde kullanırsanız (fluent api gibi) move ctor/assign'ı user-declared yazmanız gerekir ki örnek olarak yukarıda yazdım, siz yine de kullanmadan göz atın buffer logice gözümden kaçan yerler olabilir, varsa yorumda belirtirseniz düzeltiriz. Salt önlem için copy-delete yeter ama ben olsam daha makul modern bir buffer mekanizması yazardım.
mock:
İyi forumlar.
C++'da user-declared dtor yazarsanız (kaynak yöneten sınıflarda genelde yazarsınız) derleyici copy ctor ve copy assign operatorunu implicitly declare eder.
`TEMP_BUFFER` bir kaynak yönetiyor (has-a LPBUFFER), destructorunda kaynağı geri veriyor (RAII olarak bilinen, Bjarne tarafından uydurulmuş idiom).
Ama kaynak yöneten bir sınıfınız varsa ve derleyicinin yazdığı copy special member funclarını kullanırsanız senaryoya bağlı olarak başınıza farklı işler gelebilir.
Bİzim durumumuz için varsayalım ki;
C++:
TEMP_BUFFER b1, b2;
b1 = b2;gibi bir kod yazdınız; derleyicinin ürettiği copy-assign shallow copy yapar, b1'in eski buffer'ı dtoru görmeden (yani buffer_delete çağrılmadan) üstüne b2.buf yazılır. b1.buf == b2.buf olduğu için copy-ctor'da oluşacak corruption da meydana gelir. Yani assign = leak + corruption.
Copy-ctor'da gördüğüm kadarıyla sadece corruption var. İki nesne aynı `buf`'ı paylaşır, dtorda `buf` iki kez `buffer_delete`'e gider. `buffer_delete` eğer boyut poole sığıyorsa `free` çağırmıyor, buffer'ı free liste geri koyuyor. Aynı `buf` listeye iki kez push edilince mantıken `buf->next = buf` olur, yani next node aslında kendine işaret ediyor. Sonra `buffer_new` aynı `buf`'ı iki ayrı çağrıya dağıtır olası olarak heap overflow, infinite loop tarzı bir şey yaşanabilir. Klasik corruption, alakasız bir yerde patlar diye tahmin ediyorum.
En basit çözüm copy-ctor ve copy-assign operatoru explicit delete etmek amaç önlem almak, zaten aklı başında kimsenin TEMP_BUFFER'ı kopyalayacağını sanmıyorum.
C++:
TEMP_BUFFER(const TEMP_BUFFER&) = delete;
TEMP_BUFFER& operator=(const TEMP_BUFFER&) = delete;yok ben illa rule of five'a uyucam diyorsanız:
C++:
TEMP_BUFFER::TEMP_BUFFER(const TEMP_BUFFER& other)
: buf{(other.buf != nullptr) ? buffer_new(other.buf->mem_size) : nullptr}
, forceDelete{other.forceDelete}
{
if (other.buf != nullptr)
{
buffer_write(buf, other.buf->mem_data, other.buf->write_point_pos);
}
}
TEMP_BUFFER& TEMP_BUFFER::operator=(const TEMP_BUFFER& other)
{
if (this != &other)
{
if (buf != nullptr)
{
buffer_delete(buf);
}
forceDelete = other.forceDelete;
buf = (other.buf != nullptr) ? buffer_new(other.buf->mem_size) : nullptr;
if (other.buf != nullptr)
{
buffer_write(buf, other.buf->mem_data, other.buf->write_point_pos);
}
}
return *this;
}
TEMP_BUFFER::TEMP_BUFFER(TEMP_BUFFER&& other) noexcept
: buf{std::exchange(other.buf, nullptr)}
, forceDelete{std::exchange(other.forceDelete, false)}
{
}
TEMP_BUFFER& TEMP_BUFFER::operator=(TEMP_BUFFER&& other) noexcept
{
if (this != &other)
{
if (buf != nullptr)
{
buffer_delete(buf);
}
buf = std::exchange(other.buf, nullptr);
forceDelete = std::exchange(other.forceDelete, false);
}
return *this;
}Not: bu deep copy sadece yazılmış veriyi taşır, read_point ve flag kopyalanmaz, yani yarı okunmuş bir buffer'ın tam state'ini korumaz. Paket kurmada yazma için kullanıldığından sorun değil, ama bunu bilerek kullanın.
Not2: dediğim gibi hayati bir şey değil büyük ihtimalla buffer'ı hiç kopyalamamışsınızdır ama benzer diğer kaynak yöneten classlarda da benzer durumları göz önünde bulundurun.
Tek uyarı: user-declared dtor olduğunda move ctor ve move assign operator derleyici tarafından implicitly declate edilmez, yani copy'leri delete edince tip hiç taşınamaz hale gelir. Buffer'ı `std::move`'layan bir yerde kullanırsanız (fluent api gibi) move ctor/assign'ı user-declared yazmanız gerekir ki örnek olarak yukarıda yazdım, siz yine de kullanmadan göz atın buffer logice gözümden kaçan yerler olabilir, varsa yorumda belirtirseniz düzeltiriz. Salt önlem için copy-delete yeter ama ben olsam daha makul modern bir buffer mekanizması yazardım.
mock:
Linkleri görebilmek için Turkmmo Forumuna ÜYE olmanız gerekmektedir.
İyi forumlar.
En Çok Reaksiyon Alan Mesajlar
benim kartın limiti dolmuş o yüzden kendi bilgimi kullanarak yorum yapmam gerekiyor. kendi bilgimle de uzun bir inceleme yapmaya üşeniyorum. zaten bu konu hakkında çokta bilgi sahibi değilim o yüzden elimden sadece teşekkür etmek geliyor. @Larry Watterson eline sağlık teşekkürler.sıra bende
Eline sağlık, tespit bence doğru. TEMP_BUFFER burada value-semantic bir tip gibi değil, ownership taşıyan geçici bir RAII wrapper gibi davranıyor. Bu yüzden benim de ilk tercihim deep copy yazmak değil, copy ctor ve copy assignment’ı tamamen kapatmak olurdu.
TEMP_BUFFER(const TEMP_BUFFER&= delete;<br>TEMP_BUFFER& operator=(const TEMP_BUFFER&
Çünkü deep copy yazınca sınıfa “kopyalanabilir ve aynı state’i korur” anlamı yüklenmiş oluyor. Ama senin de not düştüğün gibi read point, flag/state vb. her şey birebir taşınmıyorsa bu sefer başka bir yanıltıcı davranış doğabilir. TEMP_BUFFER paket oluşturma tarafında geçici buffer gibi kullanılıyorsa copy-delete çok daha net ve güvenli.
Move tarafı için de uyarın yerinde. User-declared destructor olduğu için move ctor / move assignment otomatik gelmiyor. Copy’leri delete edince sınıf move da edilemiyorsa, ileride std::move, return value, fluent kullanım veya container içinde kullanım gibi yerlerde patlayabilir. O yüzden gerçekten taşınması gerekiyorsa explicit move yazmak mantıklı.
Bir de deep copy tercih edilecekse copy assignment kısmında küçük bir exception-safety notu var: mevcut buf önce delete edilip sonra yeni buffer alınıyor. buffer_new / buffer_write tarafında başarısızlık ihtimali varsa nesne yarım state’te kalabilir. Daha güvenli yaklaşım önce yeni buffer’ı oluşturup yazmak, her şey başarılı olduktan sonra eski buffer’ı bırakmak veya copy-and-swap tarzı gitmek olur.
Özetle TEMP_BUFFER için bence en doğru çözüm:
TEMP_BUFFER(const TEMP_BUFFER&
Uzun vadede de raw LPBUFFER sahipliğini doğrudan sınıfta taşımak yerine custom deleter’lı unique_ptr benzeri bir handle/wrapper yazmak daha temiz olur. Böylece “bu kaynak kime ait, kim release edecek, kopyalanabilir mi?” soruları koddan daha net okunur.
Kısacası sorun hayati olmasa bile tespit doğru: kaynak yöneten sınıflarda compiler’ın ürettiği copy special member’lara güvenmek genelde saatli bomba.
sıra bende
Eline sağlık, tespit bence doğru. TEMP_BUFFER burada value-semantic bir tip gibi değil, ownership taşıyan geçici bir RAII wrapper gibi davranıyor. Bu yüzden benim de ilk tercihim deep copy yazmak değil, copy ctor ve copy assignment’ı tamamen kapatmak olurdu.
TEMP_BUFFER(const TEMP_BUFFER&
Çünkü deep copy yazınca sınıfa “kopyalanabilir ve aynı state’i korur” anlamı yüklenmiş oluyor. Ama senin de not düştüğün gibi read point, flag/state vb. her şey birebir taşınmıyorsa bu sefer başka bir yanıltıcı davranış doğabilir. TEMP_BUFFER paket oluşturma tarafında geçici buffer gibi kullanılıyorsa copy-delete çok daha net ve güvenli.
Move tarafı için de uyarın yerinde. User-declared destructor olduğu için move ctor / move assignment otomatik gelmiyor. Copy’leri delete edince sınıf move da edilemiyorsa, ileride std::move, return value, fluent kullanım veya container içinde kullanım gibi yerlerde patlayabilir. O yüzden gerçekten taşınması gerekiyorsa explicit move yazmak mantıklı.
Bir de deep copy tercih edilecekse copy assignment kısmında küçük bir exception-safety notu var: mevcut buf önce delete edilip sonra yeni buffer alınıyor. buffer_new / buffer_write tarafında başarısızlık ihtimali varsa nesne yarım state’te kalabilir. Daha güvenli yaklaşım önce yeni buffer’ı oluşturup yazmak, her şey başarılı olduktan sonra eski buffer’ı bırakmak veya copy-and-swap tarzı gitmek olur.
Özetle TEMP_BUFFER için bence en doğru çözüm:
TEMP_BUFFER(const TEMP_BUFFER&
Uzun vadede de raw LPBUFFER sahipliğini doğrudan sınıfta taşımak yerine custom deleter’lı unique_ptr benzeri bir handle/wrapper yazmak daha temiz olur. Böylece “bu kaynak kime ait, kim release edecek, kopyalanabilir mi?” soruları koddan daha net okunur.
Kısacası sorun hayati olmasa bile tespit doğru: kaynak yöneten sınıflarda compiler’ın ürettiği copy special member’lara güvenmek genelde saatli bomba.
Ben yıllardır söylüyorum zaten.C++‘da asıl olay destructor değil, constructor’ın destructor’a olan bakış açısıdır.
Burada asıl problem buffer’ın frekans rezonansına girmesi.Buffer kendi kendini referanslayınca CPU cache de doğal olarak panik oluyor.
O yüzden branch predictor da yanlış tahmin yapıyor.
Ben olsam bunu constexpr virtual inline noexcept volatile yapardım.Hem compile time’da optimize olur hem runtime’da daha stabil çalışır.
Zaten Rule of Five falan eski kaldı.Ben Rule of Seven kullanıyorum.
İki kural fazladan ekleyince memory kendini yönetiyor.
Bir de buffer’ı register keyword’üyle tanımlasanız compiler direkt L1 cache’e koyuyor diye biliyorum.Böylece shallow copy olsa bile deep hissediyor.
Bence asıl sorun pointer’da değil, pointer’ın motivasyonunda.Buffer kendini sahiplenememiş. Ownership mindset’i eksik.
Bu arada std::move da aslında taşımıyor.Sadece taşımış gibi davranıyor. Gerçek taşıma RAM’in kararına bağlı.
Neyse hocam elinize sağlık.Konuyu okuyunca zaten undefined behavior’ın quantum fiziğiyle bağlantılı olduğunu bir kez daha görmüş olduk.
herkes senior dev, bence herkes anlayacaktır
Bence Larry'nin amacı tam olarak bu. Bu tarz paylaşımlarla forumun otoritesini artırıyor; Google'ın gözünde güçlenip zamanla daha elit ve kaliteli bir kullanıcı kitlesini foruma çekmek istiyor.
herkes senior dev, bence herkes anlayacaktır
Bence Larry'nin amacı tam olarak bu. Bu tarz paylaşımlarla forumun otoritesini artırıyor; Google'ın gözünde güçlenip zamanla daha elit ve kaliteli bir kullanıcı kitlesini foruma çekmek istiyor.
Katılıyorum. TEMP_BUFFER bir kaynağın (LPBUFFER) sahibi olduğu için kopyalanabilir olmaması gerekiyor. Destructor tanımlı olmasına rağmen compiler'ın ürettiği varsayılan copy constructor ve copy assignment yalnızca pointer'ı kopyalıyor (shallow copy). Bu da copy constructor senaryosunda double delete/pool corruption, copy assignment senaryosunda ise hem leak hem de double delete riski oluşturuyor.
Bence en doğru çözüm TEMP_BUFFER'ı non-copyable yapmak:
TEMP_BUFFER(const TEMP_BUFFER& = delete;<br>TEMP_BUFFER& operator=(const TEMP_BUFFER& = delete;<br>
Eğer gerçekten sahiplik aktarımı gerekiyorsa, o zaman move constructor ve move assignment eklenmeli. Deep copy ise ancak gerçekten ihtiyaç varsa ve buffer'ın tüm durumunu (read/write pointer, flag vb.) eksiksiz kopyalayabiliyorsa düşünülmeli. Aksi halde yarım bir deep copy yeni hatalara yol açabilir.
Kısacası bu sınıfın semantiği "tek sahiplik" (unique ownership) olduğundan, kopyalamayı tamamen engellemek en güvenli ve en doğru yaklaşım gibi görünüyor.
Bence en doğru çözüm TEMP_BUFFER'ı non-copyable yapmak:
TEMP_BUFFER(const TEMP_BUFFER&
= delete;<br>TEMP_BUFFER& operator=(const TEMP_BUFFER& = delete;<br>Eğer gerçekten sahiplik aktarımı gerekiyorsa, o zaman move constructor ve move assignment eklenmeli. Deep copy ise ancak gerçekten ihtiyaç varsa ve buffer'ın tüm durumunu (read/write pointer, flag vb.) eksiksiz kopyalayabiliyorsa düşünülmeli. Aksi halde yarım bir deep copy yeni hatalara yol açabilir.
Kısacası bu sınıfın semantiği "tek sahiplik" (unique ownership) olduğundan, kopyalamayı tamamen engellemek en güvenli ve en doğru yaklaşım gibi görünüyor.
Şu an konuyu görüntüleyenler (Toplam : 4, Üye: 1, Misafir: 3)
Benzer konular
