Статические указатели на уровне функций и утечки памяти

https://stackoverflow.com/questions/2136424

22-09-2019
|

Вопрос

Я написал простую библиотеку с функцией чтения строк из файла любого размера.Функция вызывается путем передачи буфера и размера, выделенного в стеке, но если строка слишком велика, инициализируется специальный буфер, выделенный в куче, и используется для передачи обратно большей строки.

Этот буфер, выделенный в куче, имеет область действия функции и объявлен статическим, конечно же, в начале инициализированным значением NULL.Я написал несколько проверок в начале функции, чтобы проверить, не является ли буфер кучи ненулевым;если это так, то предыдущая прочитанная строка была слишком длинной.Естественно, я освобождаю буфер кучи и возвращаю ему значение NULL, думая, что при следующем чтении, скорее всего, потребуется только заполнить буфер, выделенный стеком (строки длиной более 1 МБ встречаются очень редко, даже в нашем приложении!).

Я просмотрел код и довольно тщательно его протестировал, внимательно прочитав и выполнив несколько тестов.Я достаточно уверен, что сохраняется следующий инвариант:

Буфер кучи будет нулевым (и не будет утечки памяти) при возврате функции, если буфер стека — это все, что необходимо.
Если буфер кучи не равен нулю, поскольку он был необходим, он будет освобожден при следующем вызове функции (и, возможно, повторно использован, если необходимо, в следующей строке).

Но я подумал о потенциальной проблеме:Если последняя строка в файле слишком длинная, то, поскольку функция предположительно больше не вызывается, я не уверен, что у меня есть какой-либо способ освободить буфер кучи - в конце концов, он ограничен функцией.

Итак, мой вопрос: как мне освободить динамически выделенную память в статическом указателе области функции, в идеале без повторного вызова функции?(И в идеале не делать ее глобальной переменной!)

Код доступен по запросу.(Просто у меня сейчас нет доступа, извините.И я надеюсь, что вопрос достаточно общий и хорошо объяснен, чтобы он не был нужен, но во что бы то ни стало, не стесняйтесь разубеждать меня в этом понятии!)

РЕДАКТИРОВАТЬ:Я считаю, что мне следует добавить пару замечаний по поводу использования этой функции.

Эта конкретная функция используется в виде последовательного чтения строк из файла, а затем немедленного копирования в структуры POD, по одной строке на структуру.Они создаются в куче при чтении файла, и каждая из этих структур имеет указатель char, содержащий (очищенную версию) строку из файла.Чтобы они сохранились, уже должна возникнуть копия.(Это был один из главных контраргументов, приводимых во многих ответах - о нет, строку нужно КОПИРОВАТЬ, о боже мой).

Что касается многопоточности, как я уже сказал, она предназначена для последовательного использования.Нет, это не потокобезопасно, но меня это не волнует.

Но спасибо за множество ответов!Я прочитаю их более подробно, когда у меня будет время.В настоящее время я склоняюсь либо к передаче дополнительного указателя, либо к перепроектированию функции, чтобы при fgets показывает EOF, то вместо этого я мог бы просто построить там логику освобождения, и пользователю, надеюсь, не придется об этом беспокоиться.

Решение

Если есть возможность изменить функцию, я бы рекомендовал изменить сам интерфейс функции.Я знаю, что вы потратили много времени на его отладку и тестирование, но в вашей текущей реализации есть несколько проблем:

это не потокобезопасно,
пользователь не имеет контроля над данными, поэтому он должен скопировать их, если они ему понадобятся позже, скорее всего, в буфер, который будет malloc()ed, тем самым сводя на нет все преимущества, которые вы получили от выборочного использования malloc() в вашей функции,
самое главное, как вы обнаружили, необходимо предпринять специальные действия пользователем для последней длинной строки.

Ваших пользователей не должна беспокоить странность реализации вашей функции, они должны иметь возможность «просто использовать ее».

Если вы не делаете это в образовательных целях, я бы рекомендовал посмотреть эта страница, в котором есть одна реализация «чтения строки произвольной длины из потока» и ссылки на другие подобные реализации (каждая реализация немного отличается от других, поэтому вы сможете найти ту, которая вам понравится).

Судя по вашим изменениям, MT-safe не является обязательным требованием, и копирование всегда будет происходить.Итак, наиболее очевидная конструкция — одна из двух:

Позвольте пользователю указать char **, который указывает на буфер, который ваша функция выделит, используя комбинацию malloc() и realloc() (если нужно).Пользователь несет ответственность за free() это когда будет готово.Таким образом, пользователю не придется снова копировать данные, поскольку он может передать указатель на то место, где находится конечный пункт назначения данных.
вернуть char * который выделяется вашей функцией.Опять же, пользователь несет ответственность за free() это.

Оба в значительной степени эквивалентны.

Для вашей текущей реализации вы всегда можете вернуть «не конец файла», если последняя строка очень длинная и не заканчивается новой строкой.Затем пользователь снова вызовет вашу функцию, и тогда вы сможете освободить свой буфер.Лично меня бы больше устроила функция, которая позволяла бы мне читать столько строк, сколько я захочу, и не заставляла бы меня переходить к концу файла.

Другие советы

Помимо сложности освобождения этого динамически выделяемого буфера, существует еще одна потенциальная проблема.Это не потокобезопасно.Поскольку это библиотечная функция, то всегда есть вероятность, что в будущем она будет использоваться в многопоточной среде.

Вероятно, было бы лучше потребовать, чтобы вызывающая функция освобождала буфер через соответствующую библиотечную функцию.

Это все равно может быть нормально, если вы используете стандартную технику для указания конца файла (т. е. если функция чтения строки возвращает NULL).

В этом случае происходит следующее: после прочтения последней строки потребуется еще один вызов функции чтения строки, чтобы она могла вернуть NULL, указывая на то, что достигнут конец файла.В этом последнем вызове вы можете освободить буфер.

Два варианта, которые возникают сразу:

Сделайте указатель на буфер, выделенный в куче, статическим, но в пределах файла.Добавьте (статическую) функцию, которая проверяет, не является ли она нулевой, и если она не равна нулю, free() ее выполняет.Вызовите atexit(free_func) в начале программы, где free_func — статическая функция.У вас может быть некоторая глобальная процедура настройки (вызываемая main()), в которой это делается.
Не беспокойтесь об этом;память, выделенная в куче, освобождается операционной системой при выходе вашего процесса, и утечка памяти не является кумулятивной, поэтому даже если ваша программа имеет долгую жизнь, она не вызовет исключение OOM (если только у вас нет какой-либо другой ошибки).

Я предполагаю, что ваше приложение НЕ является многопоточным;в этом случае вам вообще не следует использовать статический буфер или следует использовать локальные данные потока.

Выбранный вами интерфейс делает эту проблему неразрешимой:

Клиент не должен знать, указывает ли возвращаемое значение на статическую или динамическую память.
Возвращаемое значение должно указывать на память, которая переживет вызов.
Любой звонок может оказаться последним.

Я не уверен, почему вас беспокоит эта утечка.В конце концов, если клиент читает очень длинную строку, что-то делает с ней, затем выполняет массу вычислений и выделяет память перед чтением следующей строки, у вас все еще остается большой кусок памяти, который не используется и засоряет систему.Если вас это устраивает (произвольные вычисления происходят до того, как память будет освобождена), вы можете просто признаться, что готовы сохранять мертвую память на неопределенный срок.

Если вы не можете жить с утечкой, самое простое, что можно сделать, — это расширить интерфейс, чтобы клиент мог уведомлять вашу функцию, когда клиент закончил работу с памятью.(Сейчас в контракте с клиентом говорится, что клиент владеет памятью до тех пор, пока он снова не вызовет вашу функцию, после чего право собственности возвращается к вашей функции.) Конечно, изменить интерфейс означает либо

добавление новой функции, которая потребует от вас продвижения вашего указателя, чтобы он был static но локально для модуля компиляции, или
добавление некоторого аргумента к существующей функции (или перегрузка аргумента), чтобы у вас был вызов, который означает: «Я закончил с вашей памятью, но мне не нужна еще одна строка».

Более радикальным изменением было бы переписать функцию, чтобы она использовала динамически выделяемую память на протяжении всего ее существования, постепенно увеличивая блок по мере необходимости, пока он не станет таким же большим, как самый большой блок, когда-либо прочитанный (или, возможно, округленный до следующей степени двойки).В зависимости от реальных случаев эта стратегия может потреблять меньше адресное пространство, чем хранить большой статический буфер.

В любом случае, я не уверен, что вам следует беспокоиться об этом крайнем случае.Если вы считаете, что этот случай имеет значение, отредактируйте свой вопрос, чтобы показать нам доказательства.

Вместо области действия функции укажите область действия модуля (т. е.в области файла, но статический, поэтому он не виден за пределами этого файла.Добавьте небольшую функцию, которая освобождает буфер, и используйте atexit() чтобы гарантировать, что это вызывается до выхода из программы.Альтернативно, не беспокойтесь об этом — утечка, которая происходит только один раз и автоматически освобождается при выходе из программы, не представляет особого вреда.

Однако я чувствую себя обязанным сказать, что этот дизайн звучит для меня как верный путь к катастрофе.Когда вы освобождаете буфер, практически невозможно даже предположить, используется ли он еще.Пользователь (по-видимому) должен отслеживать, куда были возвращены данные, и копировать данные в новый буфер, если (и только если) вы выделили его динамически.В многопоточной среде вам необходимо сделать внутренний указатель локальным для потока, чтобы вообще иметь хоть какой-то шанс работать правильно.Для пользователя функция может делать одно из двух совершенно разных действий: либо возвращать буфер, принадлежащий пользователю, либо возвращать буфер, принадлежащий функции, который можно безопасно использовать только путем выделения другого буфера и копирования данные в другой буфер перед повторным вызовом функции.

Я могу придумать несколько хаков, хотя оба требуют удаления статического объявления из функции.Я не могу себе представить, почему это может быть проблемой.

Используя Расширение GCC,

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

Используя С++,

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

В любом случае дизайн мне не очень нравится.В C обычно вызывающая сторона отвечает за выделение/освобождение памяти, которую ей необходимо использовать, даже если она заполнена вызываемой стороной.

Кстати, сравните с нестандартным Glibc получить линию.Он никогда не использует статическую память.

Я просто собирался прокомментировать ответ Марка, но он может показаться немного тесным.Тем не менее, этот ответ, по сути, является комментарием к его ответу, который я считаю очень хорошим и быстрым :).

Ваша функция не только не безопасна для MT, но даже без потоков интерфейс для ее правильного использования сложен.Перед повторным вызовом функции вызывающая сторона должна завершить обработку предыдущего результата.Если этот код все еще будет использоваться через два года, кто-то будет чесать затылок, пытаясь использовать его правильно...или, что еще хуже, использовать его неправильно, даже не задумываясь об этом.Этим человеком можете быть даже вы...

Предложение Марка (требование от вызывающей стороны освободить буфер) ИМХО является наиболее разумным.Но, возможно, вы не доверяете malloc и free чтобы не вызвать фрагментацию в долгосрочной перспективе или иметь какую-либо другую причину предпочесть решение со статическим буфером.В этом случае вы можете сохранить статический буфер для строк обычной длины, определить логический флаг, указывающий, занят ли в данный момент статический буфер, и задокументировать, что следующая функция (а не free) следует вызывать с адресом буфера, когда вызывающая сторона его больше не использует:

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;
}

Единственные обстоятельства, при которых срабатывают утверждения, — это обстоятельства, при которых ваша предыдущая реализация могла пойти не так, как надо, а накладные расходы очень малы по сравнению с вашим существующим решением (только переключение флага и просьба вызывающей стороне вызвать free_buffer когда он закончит, будет чище).Если утверждение в get_line в частности, триггеры, это означает, что вам все-таки нужно динамическое распределение, потому что вызывающий объект не может завершить работу с буфером в то время, когда он запрашивает другой.

Примечание:это все еще небезопасно для MT.

Лицензировано под: CC-BY-SA с атрибуция

Не связан с StackOverflow