On 14 July 2016 at 16:58, mtsio <[email protected]> wrote:
> I think I discovered what causes the crash. In apropos_in_all_indices
> (indices.c) at line 605 we are freeing up some memory (free
> (this_fb->contents;). If I delete these lines it works correctly but
> what is the root cause of that I'm not sure. The closer I got is that
> when the address of win->node->contents is 'close' to this_fb->contents
> and we free the second one we also messing up with win->node->contents.
> If the info file we search for is big these two could overlap.
>
> Is something like that possible?
Yes, I believe you are right. It's possible for win->node->contents to
point into the middle of the block pointed to by this_fb->contents. It
doesn't always happen (for example, if the character encoding has been
converted), so that could be why I didn't reproduce the crash.
The code that frees this_fb->contents is wrong. I'm fairly sure that I
put that code there in the first place: my best guess is that I wasn't
thinking about the interactive command "M-x index-apropos" and only
thinking of the "--apropos" command-line option.
I think that it is worth trying to conserve memory for an index
apropos search, because there can potentially be many manuals
installed on a system and loading them all into memory could be too
wasteful.
Measured with the "top" command (followed by "On" to sort by memory usage):
Running ginfo, followed by "M-x index-apropos RET gettext RET": the
memory in the VIRT column (virtual memory in kilobytes) is 20736.
If I comment out the lines to free the memory, recompile, and do the
same test, I get the usage up to 57952.
I've attached my work so far on a fix, which frees the file only when
the file had to be loaded. Because I haven't been able to replicate
the crash you got, I'm not sure for certain if it solves the problem.
Index: indices.c
===================================================================
--- indices.c (revision 7149)
+++ indices.c (working copy)
@@ -558,7 +558,7 @@ apropos_in_all_indices (char *search_string, int i
for (dir_index = 0; dir_menu[dir_index]; dir_index++)
{
REFERENCE **this_index, *this_item;
- FILE_BUFFER *this_fb;
+ FILE_BUFFER *this_fb, *loaded_file = 0;
this_item = dir_menu[dir_index];
if (!this_item->filename)
@@ -574,11 +574,14 @@ apropos_in_all_indices (char *search_string, int i
if (i < dir_index)
continue;
- this_fb = info_find_file (this_item->filename);
+ this_fb = check_loaded_file (this_item->filename);
if (!this_fb)
- continue;
+ this_fb = loaded_file = info_find_file (this_item->filename);
+ if (!this_fb)
+ continue; /* Couldn't load file. */
+
if (this_fb && inform)
message_in_echo_area (_("Scanning indices of '%s'..."), this_item->filename);
@@ -602,9 +605,13 @@ apropos_in_all_indices (char *search_string, int i
free (old_indices);
}
}
+
/* Try to avoid running out of memory */
- free (this_fb->contents);
- this_fb->contents = NULL;
+ if (loaded_file)
+ {
+ free (loaded_file->contents);
+ loaded_file->contents = NULL;
+ }
}
/* Build a list of the references which contain SEARCH_STRING. */
Index: nodes.c
===================================================================
--- nodes.c (revision 7149)
+++ nodes.c (working copy)
@@ -544,24 +544,17 @@ static void get_file_character_encoding (FILE_BUFF
static void forget_info_file (FILE_BUFFER *file_buffer);
static void info_reload_file_buffer_contents (FILE_BUFFER *fb);
-/* Locate the file named by FILENAME, and return the information structure
- describing this file. The file may appear in our list of loaded files
- already, or it may not. If it does not already appear, find the file,
- and add it to the list of loaded files. If the file cannot be found,
- return a NULL FILE_BUFFER *. */
+/* Try to find a file in our list of already loaded files. */
FILE_BUFFER *
-info_find_file (char *filename)
+check_loaded_file (char *filename)
{
- int i;
+ int is_fullpath, i;
FILE_BUFFER *file_buffer;
- char *fullpath;
- int is_fullpath;
/* If full path to the file has been given, we must find it exactly. */
is_fullpath = IS_ABSOLUTE (filename)
|| filename[0] == '.' && IS_SLASH(filename[1]);
- /* First try to find the file in our list of already loaded files. */
if (info_loaded_files)
{
for (i = 0; (file_buffer = info_loaded_files[i]); i++)
@@ -601,11 +594,31 @@ FILE_BUFFER *
return file_buffer;
}
}
+ return 0;
+}
+/* Locate the file named by FILENAME, and return the information structure
+ describing this file. The file may appear in our list of loaded files
+ already, or it may not. If it does not already appear, find the file,
+ and add it to the list of loaded files. If the file cannot be found,
+ return a NULL FILE_BUFFER *. */
+FILE_BUFFER *
+info_find_file (char *filename)
+{
+ FILE_BUFFER *file_buffer;
+ char *fullpath;
+ int is_fullpath;
+
+ file_buffer = check_loaded_file (filename);
+ if (file_buffer)
+ return file_buffer;
+
/* The file wasn't loaded. Try to load it now. */
/* Get the full pathname of this file, as known by the info system.
That is to say, search along INFOPATH and expand tildes, etc. */
+ is_fullpath = IS_ABSOLUTE (filename)
+ || filename[0] == '.' && IS_SLASH(filename[1]);
if (!is_fullpath)
fullpath = info_find_fullpath (filename, 0);
else
Index: nodes.h
===================================================================
--- nodes.h (revision 7149)
+++ nodes.h (working copy)
@@ -141,6 +141,8 @@ extern size_t info_loaded_files_slots;
return a NULL FILE_BUFFER *. */
FILE_BUFFER *info_find_file (char *filename);
+FILE_BUFFER *check_loaded_file (char *filename);
+
FILE_BUFFER *info_find_subfile (char *filename);
TAG *info_create_tag (void);
