[FIX] Heart (libthecore) memory leak düzeltmesi

Kaptan Yosun

Kaptan Yosun

Co-Co Admin
Moderatör
Geliştirici
Yardımsever Üye
Usta Üye
Mesaj
1.492
Çözümler
55
Beğeni
2.502
Puan
1.839
Ticaret Puanı
0
Fix sahibi metin2.dev'den
Linkleri görebilmek için giriş yap veya kayıt ol.
'dir.
Fixi aldığım asıl konu:
Linkleri görebilmek için giriş yap veya kayıt ol.


Sorun:
Heart, bir struct'a pointer olan LPHEART typedef'ini kullanır.
heart_new fonksiyonuna bir göz atalım:

p8ej1oy.webp


Görüldüğü üzere, bu fonksiyon LPHEART'a bellek ayırmak için calloc çağırır.
Ancak bu operasyon bir sorun yaratır, calloc asla bellekten salınmaz!

Bir dakika... Belleği salmamız için bizim heart_delete adında fonksiyonumuz var...
Ever var, ancak hiçbir yerde kullanılmamış.

Potansiyel çözüm:
heart_delete çağırmak yerine, LPHEART'a bellek ayırmak için otomatik olarak bellekten salınacak bir smart pointer kullanalım.

libthecore -> heart.h:
C++:
Genişlet Daralt Kopyala 
// Öncelikle raw pointeri shared ile değiştirelim
typedef struct std::shared_ptr<HEART> LPHEART;
libthecore -> heart.cpp:
C++:
Genişlet Daralt Kopyala 
// heart_new fonksiyonundan bu değişkeni silin:
LPHEART ht;

// Bu satırı:
CREATE(ht, HEART, 1);

// Bu şekilde düzenleyin:
auto ht = std::make_shared<HEART>();

Artık heart_delete fonksiyonuna ihtiyacımız olmadığı için bu fonksiyonu silebilirsiniz.
 
Forumda paylaşarak çok iyi yaptın, geçen gün gördüm paylaşayım diyordum ama unutmuşum. 😄 Çok yerinde bir düzenleme, eski haliyle kullanılsa memory leak, free ile salınsa çift serbest bırakma gibi bir duruma sebebiyet verme ihtimali var, shared_pointer gibi bir özellik varken bunu kullanmak lazım, aslında kodda buna benzer pek çok yer var, çoğunda buna benzer düzenlemeler yapılabilir, bundan örnek alıp kendiniz de bir araştırın derim, paylaşım için teşekkürler.
 
Martysama kullananlar için :D
heart.c:
Genişlet Daralt Kopyala 
#define __LIBTHECORE__
#include "stdafx.h"
#include "heart.h"
#include <time.h> // For struct timeval

extern volatile int num_events_called;

LPHEART heart_new(int opt_usec, HEARTFUNC func)
{
    LPHEART ht;

    if (!func)
    {
        sys_err("no function defined");
        return nullptr;
    }

    ht = std::make_shared<struct heart>();
    ht->func = func;
    ht->opt_time.tv_sec = 0;
    ht->opt_time.tv_usec = opt_usec;
    ht->passes_per_sec = 1000000 / opt_usec;
    gettimeofday(&ht->last_time, (struct timezone*)0);
    gettimeofday(&ht->before_sleep, (struct timezone*)0);
    return ht;
}

void heart_delete(LPHEART ht)
{
    // std::shared_ptr kullanıldığı için free() gerekmez, otomatik olarak yönetilir.
}

int heart_idle(LPHEART ht)
{
    struct timeval now, process_time, timeout, temp_time;
    int missed_pulse;

    gettimeofday(&ht->before_sleep, (struct timezone*)0);
    process_time = *timediff(&ht->before_sleep, &ht->last_time);

    /*
     * If we were asleep for more than one pass, count missed pulses and sleep
     * until we're resynchronized with the next upcoming pulse.
     */
    if (process_time.tv_sec == 0 && process_time.tv_usec < ht->opt_time.tv_usec)
    {
        missed_pulse = 0;
    }
    else
    {
        missed_pulse = process_time.tv_sec * ht->passes_per_sec;
        missed_pulse += process_time.tv_usec / ht->opt_time.tv_usec;
    }

    if (missed_pulse > 0)
    {
        gettimeofday(&ht->last_time, (struct timezone*)0);
    }
    else
    {
        /* Calculate the time we should wake up */
        temp_time = *timediff(&ht->opt_time, &process_time);
        ht->last_time = *timeadd(&ht->before_sleep, &temp_time);

        /* Now keep sleeping until that time has come */
        gettimeofday(&now, (struct timezone*)0);
        timeout = *timediff(&ht->last_time, &now);

        thecore_sleep(&timeout);
    }

    ++missed_pulse;

    if (missed_pulse <= 0)
    {
        sys_err("missed_pulse is not positive! (%d)", missed_pulse);
        missed_pulse = 1;
    }

    if (missed_pulse > (30 * ht->passes_per_sec))
    {
        sys_err("losing %d seconds. (lag occured)", missed_pulse / ht->passes_per_sec);
        missed_pulse = 30 * ht->passes_per_sec;
    }

    return missed_pulse;
}

heart.h:
Genişlet Daralt Kopyala 
#ifndef __INC_LIBTHECORE_HEART_H__
#define __INC_LIBTHECORE_HEART_H__

#include <memory>

typedef void (*HEARTFUNC) (std::shared_ptr<struct heart> heart, int pulse);

struct heart
{
    HEARTFUNC        func;

    struct timeval    before_sleep;
    struct timeval    opt_time;
    struct timeval    last_time;

    int            passes_per_sec;
    int            pulse;
};

typedef std::shared_ptr<struct heart> LPHEART;

extern LPHEART    heart_new(int opt_usec, HEARTFUNC func);
extern void    heart_delete(LPHEART ht);
extern int    heart_idle(LPHEART ht);
extern void    heart_beat(LPHEART ht, int pulses);

#endif
 

Dosya Eklentileri

Bu da Martysama'nın unique_ptr ile yaptığı revize. Hangi çözüm tercih edilmeli bilmiyorum, size kalmış.

C++:
Genişlet Daralt Kopyala 
diff --git a/asf_server/Srcs/Server/db/src/Main.cpp b/asf_server/Srcs/Server/db/src/Main.cpp
index a34e7763..617040d5 100644
--- a/asf_server/Srcs/Server/db/src/Main.cpp
+++ b/asf_server/Srcs/Server/db/src/Main.cpp
@@ -105,7 +105,7 @@ int main()
     return 1;
 }
 
-void emptybeat(LPHEART heart, int pulse)
+void emptybeat(LPHEART & heart, int pulse)
 {
     if (!(pulse % heart->passes_per_sec))
     {
diff --git a/asf_server/Srcs/Server/game/src/main.cpp b/asf_server/Srcs/Server/game/src/main.cpp
index 15f06e3d..5f6cfd69 100644
--- a/asf_server/Srcs/Server/game/src/main.cpp
+++ b/asf_server/Srcs/Server/game/src/main.cpp
@@ -208,7 +208,7 @@ namespace
 extern std::vector<TPlayerTable> g_vec_save;
 unsigned int save_idx = 0;
 
-void heartbeat(LPHEART ht, int pulse)
+void heartbeat(LPHEART & ht, int pulse)
 {
     DWORD t;
 
diff --git a/asf_server/Srcs/Server/libthecore/include/heart.h b/asf_server/Srcs/Server/libthecore/include/heart.h
index 45fb20f9..2feb6443 100644
--- a/asf_server/Srcs/Server/libthecore/include/heart.h
+++ b/asf_server/Srcs/Server/libthecore/include/heart.h
@@ -1,10 +1,11 @@
 #ifndef __INC_LIBTHECORE_HEART_H__
 #define __INC_LIBTHECORE_HEART_H__
 
+#include <memory>
 typedef struct heart    HEART;
-typedef struct heart *    LPHEART;
+using LPHEART = std::unique_ptr<heart>;
 
-typedef void (*HEARTFUNC) (LPHEART heart, int pulse);
+typedef void (*HEARTFUNC) (LPHEART & heart, int pulse);
 
 struct heart
 {
@@ -19,8 +20,8 @@ struct heart
 };
 
 extern LPHEART    heart_new(int opt_usec, HEARTFUNC func);
-extern void    heart_delete(LPHEART ht);
-extern int    heart_idle(LPHEART ht);
-extern void    heart_beat(LPHEART ht, int pulses);
+extern void    heart_delete(LPHEART & ht);
+extern int    heart_idle(LPHEART & ht);
+extern void    heart_beat(LPHEART & ht, int pulses);
 
 #endif
diff --git a/asf_server/Srcs/Server/libthecore/src/heart.c b/asf_server/Srcs/Server/libthecore/src/heart.c
index 7316972b..13a36267 100644
--- a/asf_server/Srcs/Server/libthecore/src/heart.c
+++ b/asf_server/Srcs/Server/libthecore/src/heart.c
@@ -6,16 +6,13 @@ extern volatile int num_events_called;
 
 LPHEART heart_new(int opt_usec, HEARTFUNC func)
 {
-    LPHEART ht;
-
     if (!func)
     {
     sys_err("no function defined");
     return nullptr;
     }
 
-    CREATE(ht, HEART, 1);
-
+    auto ht = std::make_unique<heart>();
     ht->func = func;
     ht->opt_time.tv_sec = 0;
     ht->opt_time.tv_usec = opt_usec;
@@ -27,10 +24,10 @@ LPHEART heart_new(int opt_usec, HEARTFUNC func)
 
 void heart_delete(LPHEART ht)
 {
-    free(ht);
+    ht.reset();
 }
 
-int heart_idle(LPHEART ht)
+int heart_idle(LPHEART & ht)
 {
     struct timeval now, process_time, timeout, temp_time;
     int missed_pulse;
 
Kaptan Yosun demiş ki:
Bu da Martysama'nın unique_ptr ile yaptığı revize. Hangi çözüm tercih edilmeli bilmiyorum, size kalmış.

C++:
Genişlet Daralt Kopyala 
diff --git a/asf_server/Srcs/Server/db/src/Main.cpp b/asf_server/Srcs/Server/db/src/Main.cpp
index a34e7763..617040d5 100644
--- a/asf_server/Srcs/Server/db/src/Main.cpp
+++ b/asf_server/Srcs/Server/db/src/Main.cpp
@@ -105,7 +105,7 @@ int main()
     return 1;
 }
 
-void emptybeat(LPHEART heart, int pulse)
+void emptybeat(LPHEART & heart, int pulse)
 {
     if (!(pulse % heart->passes_per_sec))
     {
diff --git a/asf_server/Srcs/Server/game/src/main.cpp b/asf_server/Srcs/Server/game/src/main.cpp
index 15f06e3d..5f6cfd69 100644
--- a/asf_server/Srcs/Server/game/src/main.cpp
+++ b/asf_server/Srcs/Server/game/src/main.cpp
@@ -208,7 +208,7 @@ namespace
 extern std::vector<TPlayerTable> g_vec_save;
 unsigned int save_idx = 0;
 
-void heartbeat(LPHEART ht, int pulse)
+void heartbeat(LPHEART & ht, int pulse)
 {
     DWORD t;
 
diff --git a/asf_server/Srcs/Server/libthecore/include/heart.h b/asf_server/Srcs/Server/libthecore/include/heart.h
index 45fb20f9..2feb6443 100644
--- a/asf_server/Srcs/Server/libthecore/include/heart.h
+++ b/asf_server/Srcs/Server/libthecore/include/heart.h
@@ -1,10 +1,11 @@
 #ifndef __INC_LIBTHECORE_HEART_H__
 #define __INC_LIBTHECORE_HEART_H__
 
+#include <memory>
 typedef struct heart    HEART;
-typedef struct heart *    LPHEART;
+using LPHEART = std::unique_ptr<heart>;
 
-typedef void (*HEARTFUNC) (LPHEART heart, int pulse);
+typedef void (*HEARTFUNC) (LPHEART & heart, int pulse);
 
 struct heart
 {
@@ -19,8 +20,8 @@ struct heart
 };
 
 extern LPHEART    heart_new(int opt_usec, HEARTFUNC func);
-extern void    heart_delete(LPHEART ht);
-extern int    heart_idle(LPHEART ht);
-extern void    heart_beat(LPHEART ht, int pulses);
+extern void    heart_delete(LPHEART & ht);
+extern int    heart_idle(LPHEART & ht);
+extern void    heart_beat(LPHEART & ht, int pulses);
 
 #endif
diff --git a/asf_server/Srcs/Server/libthecore/src/heart.c b/asf_server/Srcs/Server/libthecore/src/heart.c
index 7316972b..13a36267 100644
--- a/asf_server/Srcs/Server/libthecore/src/heart.c
+++ b/asf_server/Srcs/Server/libthecore/src/heart.c
@@ -6,16 +6,13 @@ extern volatile int num_events_called;
 
 LPHEART heart_new(int opt_usec, HEARTFUNC func)
 {
-    LPHEART ht;
-
     if (!func)
     {
     sys_err("no function defined");
     return nullptr;
     }
 
-    CREATE(ht, HEART, 1);
-
+    auto ht = std::make_unique<heart>();
     ht->func = func;
     ht->opt_time.tv_sec = 0;
     ht->opt_time.tv_usec = opt_usec;
@@ -27,10 +24,10 @@ LPHEART heart_new(int opt_usec, HEARTFUNC func)
 
 void heart_delete(LPHEART ht)
 {
-    free(ht);
+    ht.reset();
 }
 
-int heart_idle(LPHEART ht)
+int heart_idle(LPHEART & ht)
 {
     struct timeval now, process_time, timeout, temp_time;
     int missed_pulse;
Hepsini görmek için tıklayın...

unique_ptr, shared_ptr'a kıyasla daha performanslı ve hafif iş yüküne sahip, fakat burada shared kullanılmasını gerektirecek bir durum var mı yok mu ona bakmak lazım, eğer yoksa unique kullanmak performans açısından daha iyi olur.
 
MT2Dev demiş ki:
unique_ptr, shared_ptr'a kıyasla daha performanslı ve hafif iş yüküne sahip, fakat burada shared kullanılmasını gerektirecek bir durum var mı yok mu ona bakmak lazım, eğer yoksa unique kullanmak performans açısından daha iyi olur.
Hepsini görmek için tıklayın...
Tamamdır, çözümü Marty gibi yaptım altyapıda.
Ancak sana sormak istediğim bir şey var. Çözümü Marty gibi yapınca Visual Studio;
C++:
Genişlet Daralt Kopyala 
void emptybeat (LPHEART &heart, int pulse) // @fix17
{
    if (! (pulse % heart->passes_per_sec))    // 1ÃÊ¿¡ Çѹø
    {
    }
}
C++:
Genişlet Daralt Kopyala 
void heartbeat (LPHEART &ht, int pulse)
{
    DWORD t;

    t = get_dword_time();
    num_events_called += event_process (pulse);
    s_dwProfiler[PROF_EVENT] += (get_dword_time() - t);

    t = get_dword_time();

    // 1Ãʸ¶´Ù
    if (! (pulse % ht->passes_per_sec))
    {
        if (!g_bAuthServer)
        {
            TPlayerCountPacket pack;
            pack.dwCount = DESC_MANAGER::instance().GetLocalUserCount();
            db_clientdesc->DBPacket (HEADER_GD_PLAYER_COUNT, 0, &pack, sizeof (TPlayerCountPacket));
        }
        else
        {
            DESC_MANAGER::instance().ProcessExpiredLoginKey();
            DBManager::instance().FlushBilling();
            /*
               if (!(pulse % (ht->passes_per_sec * 600)))
               DBManager::instance().CheckBilling();
             */
        }

        {
            int count = 0;
            itertype (g_sim) it = g_sim.begin();

            while (it != g_sim.end())
            {
                if (!it->second->IsCheck())
                {
                    it->second->SendLogin();

                    if (++count > 50)
                    {
                        sys_log (0, "FLUSH_SENT");
                        break;
                    }
                }

                it++;
            }

            if (save_idx < g_vec_save.size())
            {
                count = MIN (100, g_vec_save.size() - save_idx);

                for (int i = 0; i < count; ++i, ++save_idx)
                {
                    db_clientdesc->DBPacket (HEADER_GD_PLAYER_SAVE, 0, &g_vec_save[save_idx], sizeof (TPlayerTable));
                }

                sys_log (0, "SAVE_FLUSH %d", count);
            }
        }
    }

    //
    // 25 PPS(Pulse per second) ¶ó°í °¡Á¤ÇÒ ¶§
    //

    // ¾à 1.16Ãʸ¶´Ù
    if (! (pulse % (passes_per_sec + 4)))
    {
        CHARACTER_MANAGER::instance().ProcessDelayedSave();
    }

    // ¾à 5.08Ãʸ¶´Ù
    if (! (pulse % (passes_per_sec * 5 + 2)))
    {
        ITEM_MANAGER::instance().Update();
        DESC_MANAGER::instance().UpdateLocalUserCount();
    }

    s_dwProfiler[PROF_HEARTBEAT] += (get_dword_time() - t);

    DBManager::instance().Process();
    AccountDB::instance().Process();
    CPVPManager::instance().Process();

    if (g_bShutdown)
    {
        if (thecore_pulse() > g_shutdown_disconnect_pulse)
        {
            const DESC_MANAGER::DESC_SET& c_set_desc = DESC_MANAGER::instance().GetClientSet();
            std::for_each (c_set_desc.begin(), c_set_desc.end(), ::SendDisconnectFunc());
            g_shutdown_disconnect_pulse = INT_MAX;
        }
        else if (thecore_pulse() > g_shutdown_disconnect_force_pulse)
        {
            const DESC_MANAGER::DESC_SET& c_set_desc = DESC_MANAGER::instance().GetClientSet();
            std::for_each (c_set_desc.begin(), c_set_desc.end(), ::DisconnectFunc());
        }
        else if (thecore_pulse() > g_shutdown_disconnect_force_pulse + PASSES_PER_SEC (5))
        {
            thecore_shutdown();
        }
    }
}

Gibi bazı fonksiyonları static yapabilirsin diyor. Sence yapmanın sakıncası var mı? Visual Studio'nun bu tarz önerileri güvenli mi?
 
Kaptan Yosun demiş ki:
Tamamdır, çözümü Marty gibi yaptım altyapıda.
Ancak sana sormak istediğim bir şey var. Çözümü Marty gibi yapınca Visual Studio;
C++:
Genişlet Daralt Kopyala 
void emptybeat (LPHEART &heart, int pulse) // @fix17
{
    if (! (pulse % heart->passes_per_sec))    // 1ÃÊ¿¡ Çѹø
    {
    }
}
C++:
Genişlet Daralt Kopyala 
void heartbeat (LPHEART &ht, int pulse)
{
    DWORD t;

    t = get_dword_time();
    num_events_called += event_process (pulse);
    s_dwProfiler[PROF_EVENT] += (get_dword_time() - t);

    t = get_dword_time();

    // 1Ãʸ¶´Ù
    if (! (pulse % ht->passes_per_sec))
    {
        if (!g_bAuthServer)
        {
            TPlayerCountPacket pack;
            pack.dwCount = DESC_MANAGER::instance().GetLocalUserCount();
            db_clientdesc->DBPacket (HEADER_GD_PLAYER_COUNT, 0, &pack, sizeof (TPlayerCountPacket));
        }
        else
        {
            DESC_MANAGER::instance().ProcessExpiredLoginKey();
            DBManager::instance().FlushBilling();
            /*
               if (!(pulse % (ht->passes_per_sec * 600)))
               DBManager::instance().CheckBilling();
             */
        }

        {
            int count = 0;
            itertype (g_sim) it = g_sim.begin();

            while (it != g_sim.end())
            {
                if (!it->second->IsCheck())
                {
                    it->second->SendLogin();

                    if (++count > 50)
                    {
                        sys_log (0, "FLUSH_SENT");
                        break;
                    }
                }

                it++;
            }

            if (save_idx < g_vec_save.size())
            {
                count = MIN (100, g_vec_save.size() - save_idx);

                for (int i = 0; i < count; ++i, ++save_idx)
                {
                    db_clientdesc->DBPacket (HEADER_GD_PLAYER_SAVE, 0, &g_vec_save[save_idx], sizeof (TPlayerTable));
                }

                sys_log (0, "SAVE_FLUSH %d", count);
            }
        }
    }

    //
    // 25 PPS(Pulse per second) ¶ó°í °¡Á¤ÇÒ ¶§
    //

    // ¾à 1.16Ãʸ¶´Ù
    if (! (pulse % (passes_per_sec + 4)))
    {
        CHARACTER_MANAGER::instance().ProcessDelayedSave();
    }

    // ¾à 5.08Ãʸ¶´Ù
    if (! (pulse % (passes_per_sec * 5 + 2)))
    {
        ITEM_MANAGER::instance().Update();
        DESC_MANAGER::instance().UpdateLocalUserCount();
    }

    s_dwProfiler[PROF_HEARTBEAT] += (get_dword_time() - t);

    DBManager::instance().Process();
    AccountDB::instance().Process();
    CPVPManager::instance().Process();

    if (g_bShutdown)
    {
        if (thecore_pulse() > g_shutdown_disconnect_pulse)
        {
            const DESC_MANAGER::DESC_SET& c_set_desc = DESC_MANAGER::instance().GetClientSet();
            std::for_each (c_set_desc.begin(), c_set_desc.end(), ::SendDisconnectFunc());
            g_shutdown_disconnect_pulse = INT_MAX;
        }
        else if (thecore_pulse() > g_shutdown_disconnect_force_pulse)
        {
            const DESC_MANAGER::DESC_SET& c_set_desc = DESC_MANAGER::instance().GetClientSet();
            std::for_each (c_set_desc.begin(), c_set_desc.end(), ::DisconnectFunc());
        }
        else if (thecore_pulse() > g_shutdown_disconnect_force_pulse + PASSES_PER_SEC (5))
        {
            thecore_shutdown();
        }
    }
}

Gibi bazı fonksiyonları static yapabilirsin diyor. Sence yapmanın sakıncası var mı? Visual Studio'nun bu tarz önerileri güvenli mi?
Hepsini görmek için tıklayın...

VS veya GCC/Clang önerileri eğer kullandığın derleyici en güncel sürümüyse oldukça önemlidir, yol gösterir ve çok yüksek oranda güvenlidir (ekstrem senaryolar hariç). Burada verdiği önerinin sebebi basit kısaca, en kolay yoldan bahsetmem gerekirse; bir fonksiyonu static olarak belirlemek onun global olarak değil sadece bulunduğu dosyada lokal olarak tanımlanmasını sağlar, örneğin emptybeat fonksiyonu sadece tek bir dosyada ve onun header dosyasında tanımlı değil mi, bunu static olarak kullanırsan, derleyiciye bunun lokal bir fonksiyon olduğunu ve sadece bu dosyada kullanıldığını belirtip onu globale sokmazsın, bunun avantajı daha iyi optimizasyondur çünkü derleyici bunun sadece tek bir yerde kullanılacağını bilir, o yüzden bu öneriyi aldığında yapman gereken şey şu; bunu önerdiği fonksiyon adını (bu senaryoda emptybeat mesela) src içerisinde arat, sadece tek bir dosyada ve header dosyasında kullanılıyorsa rahatlıkla static tanımlayabilirsin.
 
Uyarı: Bu konu açıldığından bu yana baya zaman geçmiş.
Muhtemelen daha fazla tartışma gerekli değildir ki bu durumda yeni bir konu başlatmayı öneririz. Eğer yine de cevabınızın gerekli olduğunu düşünüyorsanız buna rağmen cevap verebilirsiniz.

Benzer konular

bckntr
67 Adet Fix
Yanıt
3
Gösterim
2K
Tengrist
Tengrist
Kaptan Yosun
[C++] Yüksek miktarda Stack bellek kullanan bazı fonksiyonları optimize edin
Yanıt
6
Gösterim
622
Raviel
Raviel
Whistle
Metin2 Server Source Libserverkey Kaldırma
Yanıt
13
Gösterim
4K
MEKA
MEKA
elchikapu
  • Kilitli
VisibilityCallBack Memory Leak Fixed
Yanıt
6
Gösterim
1K
Kaiser
Kaiser
Mitachi
[Fix] "when login" trigger before affect initialization
Yanıt
9
Gösterim
660
CuRSED
CuRSED
Geri
Üst