7
votes

Mon programme fuit avec des ressources détenues par Boost :: Shared_ptr

Je ne vois pas pourquoi mon programme fuit, peut-être que vous pouvez le repérer. XXX PRE>

Ma classe contient cette fenêtre de la fenêtreMap fonctionne correctement, mais la dérappement ne semble pas fonctionner correctement . Le destructeur de classe appelle cette fonction pour effacer la carte - qui devrait libérer le Shared_Ptr's, les supprimant ainsi, non? :) p>

BOOL CALLBACK EnumWindowsCallback( HWND hWnd )
{
    // adds this window to the WindowMap, along with its title text

    BOOL        bRetVal         = FALSE;
    int         nTextLen        = 0;
    char*       sWindowText     = NULL;     
    StringPtr   strPtr;     

    if( ! ::IsWindow( hWnd ) )
        return FALSE;

    nTextLen = GetWindowTextLength( hWnd );
    if( ! nTextLen )
        return TRUE;

    sWindowText = new char[nTextLen + 1];
    if( sWindowText )
    {
        GetWindowTextA( hWnd, sWindowText, nTextLen );

        strPtr = StringPtr(new std::string(sWindowText));

        m_Windows.insert( WMapPair(hWnd, strPtr) );

        delete [] sWindowText;

        sWindowText = NULL;
        bRetVal     = TRUE;
    }

    return bRetVal;
}


22 commentaires

Je ne vois pas une cause évidente de la fuite. (Je suis sûr que d'autres le trouveront.) Cependant, je pense que vous faites des choses plus compliquées que nécessaire: STD :: String va automatiquement nettoyer ses ressources lorsque son destructeur est appelé. Pourquoi ne pas simplement utiliser une carte de STD :: map ?


Ceci est faux stylistiquement: stringptr (nouveau std :: string (swindowtext)) . Chaque nouvel objet attribué dynamiquement doit être initialement appartenant à un pointeur intelligent nommé . Pour plus, lisez Boost Shared_PTR Meilleures pratiques . En outre, envisagez d'utiliser un std :: vecteur au lieu d'attribuer de manière dynamique le tableau vous-même. Je ne pense pas que ce soit de ce que ce soit la cause de votre problème spécifique, cependant.


Quelle version de quel compilateur?


@ERIC PI: Le rappel allouerait la STD :: String sur la pile, donc dès que le rappel du rappel, la chaîne STD :: serait libérée, d'où la nécessité d'une chaîne allouée en tas.


@James McNellis, merci pour le lien, j'ai fait la mise à jour, mais cela n'a fait aucune différence (j'ai noté que vous avez dit que ce n'était pas la cause de mon problème cependant). Je vais regarder en utilisant un std :: vecteur une fois que je reçois la question de la fuite de mémoire triée!


@FreeFallRR: Non; quand vous avez un std :: map m; et vous faites un m.insert (std :: make_pair (0, std :: string ("Bonjour World "))); , une copie du temporaire std :: string (" Hello World ") est inséré dans le std :: map .


Swindowtext: Ne peut jamais être nul. Ne vous inquiétez pas de vérifier. De plus, les objets neufs / Supprimer utilisent un objet approprié qui le fait pour vous (comme dans ce cas une chaîne). Aussi curieux pourquoi vous avez même besoin d'allouer de manière dynamique la chaîne. Créez et copiez simplement sur la carte.


@Martin je suppose que c'est une vérification paresseuse pour l'échec de l'allocation de mémoire. J'alloque de manière dynamique la chaîne car sa longueur n'est pas connue tant que l'exécution est-elle facile à faire. Je suis intéressé à apprendre à fournir un vecteur de char comme un argument à GetWindowtexta (), mais il est deuxième à mon problème de la fuite de mémoire lorsqu'il réalise la carte.


@Martin - Je viens de vérifier, selon MSDN, le nouvel opérateur peut échouer et retourner zéro, donc mon chèque était valide. msdn.microsoft.com/en-us/library/kewsb8ba.aspx - "En cas d'échec, le nouveau retourne zéro ou jette une exception"


@James - Je prends votre point à utiliser un std :: ficelle au lieu d'un boost :: Shared_ptr . Je suppose que ma conclusion (incorrecte) pourrait être facile à faire - qu'une chaîne allouée de pile pouvait être sans signification après que la pile se déroule; Je ne savais pas que cela survivrait lorsqu'il est ajouté à une carte. Je vais aller avec cette solution, comme je me déchirai mes cheveux sur cette question trop longtemps maintenant!


@James - Que diriez-vous d'un exemple de comment utiliser un vecteur de char comme 2e argument pour GetWindowtext ( msdn.microsoft.com/en-us/library/ms633520 (v = vs.85) .aspx )


Êtes-vous sûr de ne pas avoir de référence circulaire? C'est ce que Stimuler :: faible_ptr est pour.


Vous devez redimensionner le vecteur de la taille suffisante, vous pouvez obtenir un pointeur à son élément initial à l'aide de & V [0] . Par exemple: std :: vecteur v (ntextlen + 1); Getwindowtext (hwnd, & v [0], ntextlen);


@FreeFallRR - Vous n'utilisez pas l'opérateur Nouveau que vous avez lié à, mais l'opérateur neuf (une chose différente, malgré le nom similaire). Lisez ici Les nouveaux et supprits des opérateurs à la place.


@Bo Persson - c'est étrange. J'ai regardé votre lien. Il vous amène à une page qui relie directement mon lien pour le nouvel opérateur !!!


@FreeFallRR - Lorsque vous écrivez Nouveau X dans votre programme, c'est le nouvel opérateur que je suis lié. Il est dit là que cela lancera un std :: bad_alloc s'il échoue.


Je ne conteste pas que cela peut lancer un STD :: Bad_alloc quand il échoue. Toutefois, la page I liée à (et la page que vous êtes également liée indirectement liée à) indique que "si infructueux, nouveau retourne zéro ou jette une exception". Donc, il peut faire soit.


@Sam Miller - En tout cas, je viens de rester avec la suggestion de James Mcnellis de STD :: make_pair (hwnd, std :: string (BLEH))


@FreeFallRR - Non, ça ne peut pas! :-) La page que vous liez est pour Opérateur Nouveau Un opérateur que vous pouvez remplacer pour avoir une allocation de mémoire spéciale pour un type. Ma page concerne le Nouveau dans une déclaration comme Nouveau X . Même s'ils utilisent tous les deux le mot nouveau , ils signifient différentes choses. Et je peux vous assurer que nouveau x ne retournera pas NULL, il lancera une exception.


Je cite la page MSDN pour "nouvel opérateur" - si infructueuse, nouvelle renvoie zéro ou jette une exception; Voir «Les nouveaux et Supprimer les opérateurs» pour plus d'informations. Le lien des opérateurs neufs et Supprimer vous amène à votre page. La documentation MSDN est définitivement trompeuse dans ce cas. Je vais peut-être le regarder sous Windows via C / C ++ ou similaire ...


@FreeFallRR: Nouveau retournera 0 si vous compilez avec des exceptions désactivées. Tant que vous compilez avec des exceptions allumées (et si vous ne le faites pas, n'utilisez pas la STL), il sera toujours jeter au cas où il ne peut pas allouer.


@FreeFallRR: Neuf fonctionne avec succès ou jette une exception. Si vous utilisez la version sans jette, cela peut renvoyer NULL, mais vous n'utilisez pas cela. Vérification de NULL n'est pas seulement inutile qu'il rend le code plus difficile à lire et à entretenir.


3 Réponses :


2
votes

Il y avait (est?) Un bug dans vs2010 std :: vecteur <> La mémoire provoque une fuite sous certaines circonstances (voir ici ). Afaik, il a été corrigé dans VS2010 SP1, mais je ne suis pas 100% positif.


3 commentaires

Merci pour la tête, je n'étais pas au courant de ça! J'ai installé VS2010 SP1 et mon problème semble être avec une carte plutôt qu'avec un vecteur.


Rien n'empêche mappe à partir de Vecteur en interne.


Je peux confirmer que cela a été corrigé dans SP1.



0
votes

Je suis allé avec la suggestion de James:

@freeFallRR: Non; Quand vous avez une STD :: map m; Et vous faites un m.insert (std :: make_pair (0, std :: String ("Hello World"))) ;, une copie de la STD temporaire :: String ("Hello World") est insérée dans le STD: :carte. - James McNellis


0 commentaires

0
votes

ne pas répondre à votre question (comme je peux; t voir des problèmes) mais quelques points:

typedef std::map  < HWND, std::string >     WindowMap;
typedef WindowMap::value_type               WMapPair;    // In the future it may not be a pair.
                                                         // Or you may change the type of WindowMap
                                                         // Make it so the value is based on the container.
                                                         // Done automatically.

// this callback populates the WindowMap (m_Windows) by adding a WMapPair each time
BOOL CALLBACK EnumWindowsCallback( HWND hWnd )
{
    // adds this window to the WindowMap, along with its title text

    // Declare variables at there first usage point.
    // There is no need to clutter the top of the function 
    // With usless variables that may never be used.

    if( ! ::IsWindow( hWnd ) )
        return FALSE;

    int nTextLen = GetWindowTextLength( hWnd );
    if( ! nTextLen )
        return TRUE;

    // Use a vector. Dynamically allocating memory is dangerious and not
    // exception safe (unless you use a smart pointer (or in this case a container))
    std::vector<char>  sWindowText(nTextLen + 1);
    GetWindowTextA( hWnd, &sWindowText[0], nTextLen );

    // No need for shared pointers. Just put the string in the map.
    m_Windows.insert( WMapPair(hWnd, std::string(sWindowText.begin(),
                                                 sWindowText.begin()+ nTextLen)));

    return TRUE;
}


1 commentaires

Ouais, je suis allé avec ce type d'approche (par exemple, suggestion de James comme indiqué précédemment)