pointeurs scope de fonction statique et les fuites de mémoire

https://stackoverflow.com/questions/2136424

22-09-2019
|

Question

J'ai écrit un simple fichier de bibliothèque avec une fonction pour les lignes de lecture d'un fichier de toute taille. La fonction est appelée par passage dans une mémoire tampon allouée pile et la taille, mais si la ligne est trop grande, une mémoire tampon allouée spécial est initialisé et utilisé pour passer en arrière d'une ligne plus grande.

Ce mémoire tampon alloué est fonction scope et déclarée statique, initialisé à NULL au début du cours. Je l'ai écrit dans certains contrôles au début de la fonction, pour vérifier si le tampon de tas est non nul; si tel est le cas, la lecture de la ligne précédente était trop long. Bien entendu, je libérer le tampon du tas et le ramener à NULL, pensant que la prochaine lecture sera probablement seulement besoin de remplir la mémoire tampon de la pile allouée (il devrait être très rare de voir des lignes plus 1Mo long, même dans notre application!).

Je suis allé sur le code et testé assez à fond, à la fois en lisant attentivement et en exécutant quelques tests. Je suis raisonnablement confiant que l'invariant suivant est maintenu:

La mémoire tampon sera nulle (et ne coule pas toute la mémoire) sur le retour de fonction si le tampon de la pile est tout ce qui est nécessaire.
Si le tampon de tas est non nul, parce qu'il était nécessaire, il sera libéré lors du prochain appel de fonction (et peut-être réutilisé si nécessaire sur cette ligne suivante).

Mais j'ai pensé à un problème potentiel: Si la dernière ligne dans un fichier est trop long, alors, puisque la fonction est probablement pas appelé à nouveau, je ne suis pas sûr d'avoir un moyen de libérer le buffer-- tas il est fonction scope, après tout.

Alors, ma question est, comment puis-je faire pour libérer de la mémoire allouée dynamiquement dans un pointeur statique scope fonction, idéalement sans appeler à nouveau la fonction? (Et idéalement, sans en faire une variable globale, que ce soit!)

code disponible sur demande. (Je viens pas eu accès maintenant, excusez-moi. Et j'espère que la question est suffisamment générale et bien expliqué pour qu'elle ne soit pas nécessaire, mais par tous les moyens se sentent à me détromper de cette notion!)

EDIT: Je crois que je devrais ajouter quelques notes sur l'utilisation de la fonction

Cette fonction particulière est utilisée sous la forme de lignes étant lues en série à partir d'un fichier, puis immédiatement copié dans POD structs, une ligne par struct. Ceux-ci sont créés sur le tas que le fichier est lu, et chacun de ces struct a un pointeur char contenant (une version nettoyée de) une ligne à partir du fichier. Pour que ces persistent, une copie a déjà de se produire. (Ce fut l'un des grands arguments contraires élevés dans beaucoup de answers-- oh non, la ligne doit duplicable, oh me Dearie).

En ce qui concerne multithreading, comme je l'ai dit est conçu pour être utilisé en série. Non, il est pas sûre, mais je ne se soucient pas.

Merci pour la multitude de réponses, bien que! Je les lis plus à fond quand je reçois le temps. À l'heure actuelle, je me penche vers soit le passage d'un pointeur supplémentaire autour ou redessiner la fonction de sorte que lorsque fgets montre EOF, alors je pourrais simplement construire la logique libératrice il à la place et l'utilisateur, espérons pas besoin de se soucier.

La solution

Si vous pouvez changer la fonction, je recommanderais de changer l'interface de la fonction elle-même. Je sais que vous avez passé beaucoup de temps à déboguer et de tester, mais il y a quelques problèmes avec votre implémentation actuelle:

il est thread-safe,
l'utilisateur n'a aucun contrôle sur les données, donc il doit copier s'il en a besoin plus tard, le plus probable dans un tampon qui va être malloc()ed, annulant ainsi tout avantage que vous avez obtenu par l'utilisation sélective de malloc() dans votre fonction,
le plus important, comme vous l'avez découvert, une action particulière doit être prise par l'utilisateur pour une longue dernière ligne.

Vos utilisateurs ne doivent pas être préoccupés par la bizarrerie de la mise en œuvre de votre fonction, ils devraient être en mesure de « juste utiliser ».

À moins que vous le faites à des fins éducatives, je recommanderais regarder cette , qui a une mise en œuvre de « la lecture d'une ligne arbitraire à long d'un cours d'eau », et des liens vers d'autres telles mises en œuvre (chaque mise en œuvre est un peu différent des autres, donc vous devriez être en mesure de trouver celui que vous aimez) .

Sur la base de votre édition, MT-safe n'est pas une exigence et une copie va se passer toujours. Alors, est l'un des deux la conception la plus évidente:

Laissez l'utilisateur fournir un char **, qui pointe vers un tampon que votre fonction attribuera, en utilisant une combinaison de malloc() et realloc() (le cas échéant). Il est de la responsabilité de l'utilisateur de ce free() lorsque vous avez terminé. De cette façon, l'utilisateur n'a pas à copier à nouveau les données, car il peut passer un pointeur à l'endroit où la destination finale des données est.
un retour char * qui est allouée par votre fonction. Encore une fois, il est de la responsabilité de l'utilisateur de free() il.

Les deux sont à peu près équivalents.

Pour votre implémentation actuelle, vous pouvez toujours revenir « pas la fin du fichier » si la dernière ligne est très longue, et ne se termine pas dans une nouvelle ligne. Ensuite, l'utilisateur va appeler à nouveau votre fonction, et ensuite vous pouvez libérer votre tampon. Personnellement, je serais plus heureux avec une fonction qui me permet de lire autant de lignes que je veux, et non me forcer à aller à la fin du fichier.

Autres conseils

En plus de la difficulté à libérer cette mémoire tampon allouée dynamiquement, il existe un autre problème potentiel. Il est pas thread-safe. Comme il est une fonction de bibliothèque, puis il y a toujours la possibilité qu'il sera utilisé dans un environnement multi-thread dans l'avenir.

Il serait probablement préférable d'exiger la fonction appelante pour libérer la mémoire tampon via une fonction de bibliothèque associée.

Cela pourrait encore être d'accord si vous utilisez la technique standard pour indiquer la fin de fichier (i.e. avez-vous le retour de fonction de lecture en ligne NULL).

Qu'est-ce qui se passe dans ce cas est que, après la dernière ligne est lue, un appel à plus de votre fonction de lecture en ligne est nécessaire pour qu'il puisse retourner NULL pour indiquer que la fin du fichier a été atteinte. Dans ce dernier appel, vous pouvez vous libérer tampon.

Deux choix qui se produisent immédiatement:

Faire le pointeur sur le buffer alloué tas statique mais le fichier SCOPED. Ajout d'une fonction (statique) qui vérifie si elle est non nulle et si elle est pas libre () nulle s il. Appelez atexit (free_func) au début du programme, où free_func est la fonction statique. Vous pouvez avoir une routine de configuration globale (caled par main ()) lorsque cela est fait.
Ne vous inquiétez pas à ce sujet; la mémoire de tas alloué est libéré par le système d'exploitation lorsque vos sorties de processus, et la fuite de mémoire ne sont pas cumulables, même si votre programme a une longue vie, il ne soulèvera pas une exception OOM (sauf si vous avez un autre bug).

Je suppose que votre application n'est pas multithread; dans ce cas, vous ne devriez pas utiliser un tampon statique du tout, ou vous devez utiliser les données locales de thread.

L'interface que vous avez choisi en fait un problème insoluble:

Le client ne doit pas savoir si les points de valeur de retour à la mémoire statique ou dynamique.
La valeur de retour doit pointer vers la mémoire qui survivrait l'appel.
Tout appel pourrait être le dernier.

Je ne sais pas pourquoi vous êtes troublé par cette fuite. Après tout, si le client lit une ligne très longue, fait quelque chose avec la ligne, puis fait une tonne de calcul et de répartition avant de lire la ligne suivante, vous avez encore un gros morceau de la mémoire assis autour utilisé, le colmatage du système. Si ce OK avec vous (calcul arbitraire a lieu avant que la mémoire est régénérées), vous pouvez simplement vous que vous fess êtes prêt à conserver indéfiniment la mémoire morte.

Si vous ne pouvez pas vivre avec la fuite, la chose la plus simple à faire est d'élargir l'interface afin que le client peut informer votre fonction lorsque le client se fait avec la mémoire. (En ce moment le contrat avec le client dit que le client est propriétaire de la mémoire jusqu'à ce qu'elle appelle votre fonction à nouveau, au cours de laquelle la propriété point revient à votre fonction.) Bien sûr, pour changer l'interface signifie soit

l'ajout d'une nouvelle fonction, qui vous demandera de promouvoir votre pointeur à static local, mais à l'unité de compilation, ou
ajouter un argument à la fonction existante (ou la surcharge d'un argument) de sorte que vous avez un appel qui signifie « Je suis fait avec votre mémoire, mais je ne veux pas une autre ligne ».

Un changement plus radical serait de réécrire la fonction d'utiliser la mémoire allouée dynamiquement tout au long de sa durée de vie, élargissant progressivement le bloc au besoin jusqu'à ce qu'il soit aussi grand que le plus grand bloc jamais lu (ou peut-être arrondi à la puissance suivante de deux ). Selon les cas réels de cette stratégie peut consommer moins espace d'adressage que de garder un grand tampon statique.

Dans tous les cas, je ne suis pas convaincu que vous devriez être inquiétant dans ce cas de coin. Si vous pensez que cette question est importante de cas, s'il vous plaît modifier votre question pour nous montrer la preuve.

Au lieu de la portée de la fonction, donner Module portée (par exemple à la portée du fichier, mais statique, il est donc pas visible en dehors de ce fichier. Ajouter une petite fonction qui permet de libérer la mémoire tampon, et utiliser atexit() pour assurer que ce qu'on appelle avant la sortie du programme. Alternative, ne vous inquiétez pas à ce sujet - une fuite qui se produit qu'une seule fois, et est libéré automatiquement les sorties du programme ne sont pas particulièrement nocif

Je me sens obligé de dire que la conception me semble comme une recette pour un désastre si. Lorsque vous libérez le tampon, il n'y a pratiquement aucun moyen de deviner, même si elle peut encore être utilisé. L'utilisateur (apparemment) doit garder une trace de l'endroit où les données sont renvoyées, et copier les données vers un nouveau tampon si (et seulement si) vous avez alloué une dynamique. Dans un environnement multi-thread, vous devez faire le fil local pointeur interne pour avoir une chance de fonctionner correctement à tous. Pour l'utilisateur, la fonction peut faire l'une des deux choses entièrement différentes - soit retourner un tampon qui appartient à l'utilisateur ou le retour d'un tampon qui appartient à la fonction, et ne peut être utilisé en toute sécurité en allouant un autre tampon, et la copie du des données dans l'autre tampon avant que la fonction est appelée de nouveau.

Il y a quelques hacks que je peux penser, même si les deux nécessitent le déplacement de la déclaration statique de la fonction. Je ne peux pas imaginer pourquoi ce serait un problème.

En utilisant un extension de GCC ,

static char *buffer;
void use_buffer(size_t n) {
    buffer = realloc(buffer, n);
}
void cleanup_buffer() __attribute__((destructor)) {
    free(buffer);
}

Utilisation de C ++,

static char *buffer;
static class buffer_guard {
    ~buffer_guard() { free(buffer); }
} my_buffer_guard;

Dans tous les cas, je ne aime pas vraiment la conception. En C, habituellement l'appelant est responsable de l'allocation / libération de la mémoire qu'il a besoin d'utiliser, même si elle est remplie par un callee.

BTW, comparez avec getline glibc . Il utilise jamais la mémoire statique.

Je voulais simplement faire des commentaires ci-dessous la réponse de Mark, mais il peut se sentir un peu à l'étroit. Pourtant, cette réponse est en substance un commentaire sur sa réponse, que je trouve très bien en plus d'être rapide.)

Non seulement votre fonction ne MT-sûr, mais même sans fils, l'interface à l'utiliser est compliquée correctement. L'appelant doit avoir terminé avec le résultat précédent avant d'appeler à nouveau la fonction. Si ce code est encore en cours d'utilisation de deux ans à partir de maintenant, quelqu'un va se gratter la tête en essayant de l'utiliser à droite ... ou pire, utilisez mal, sans même y penser. Cette personne pourrait même être vous ...

La suggestion de Mark (nécessitant l'appelant à libérer la mémoire tampon) est à mon humble avis le plus raisonnable. Mais peut-être que vous ne faites pas confiance malloc et free de ne pas provoquer la fragmentation à long terme, ou avoir une autre raison de préférer la solution tampon statique. Dans ce cas, vous pouvez garder la mémoire tampon statique pour les lignes de longueur ordinaire, définir un indicateur booléen qui indique si le tampon statique est actuellement occupé, et le document que la fonction suivante (et non free) doit être appelée à l'adresse du tampon lorsque l'appelant ne l'utilise plus:

char static_buffer[512];
int buffer_busy;

void free_buffer(char *p)
{
  if (p == static_buffer)
  {
     assert(buffer_busy);
     buffer_busy=0;
  }
  else free(p);
}

char *get_line(...)
{
  char *result;
  if (..short line..)
  {
     result = static_buffer;
     assert(!buffer_busy);
     buffer_busy=1;
  }
  else result = malloc(...);
  ...
  return result;
}

Les seules circonstances dans lesquelles les affirmations vont déclencher des circonstances dans lesquelles votre mise en œuvre précédente aurait silencieusement ont mal tourné, et les frais généraux est très faible par rapport à votre solution existante (uniquement basculer le drapeau, et de demander à l'appelant d'appeler free_buffer lorsque il est fini, ce qui est plus propre). Si l'affirmation contenue dans get_line dans les déclencheurs particuliers, cela signifie que vous avez besoin l'allocation dynamique après tout, parce que l'appelant n'a pas pu être terminé avec un tampon au moment où il demandait un autre.

Note:. Ce n'est pas encore MT-safe

Licencié sous: CC-BY-SA avec attribution

Non affilié à StackOverflow