Subject: bug#46256: [feature/native-comp] AOT eln files ignored if run from build tree

From: Andrea Corallo <[email-masked]>
Date: Tue, 09 Mar 2021 19:38:15 +0000

Eli Zaretskii <[email-masked]> writes:

>> From: Andrea Corallo <[email-masked]>
>> Cc: [email-masked], [email-masked], [email-masked]
>> Date: Tue, 09 Mar 2021 14:55:40 +0000
>> 
>> cc-*.el files for instance have more than one direct call to load.  IIRC
>> one of the analyzed cases was cc-mode related (certanly one I observed).
>> 
>> >> > What are the reasons for unloading a .eln file which was once loaded
>> >> > into a session?
>> >>
>> >> All the functions defined in it are not anymore reachable (read all
>> >> symbols functions are makunbound or defined to some other function).
>> >
>> > That means someone did unload-feature, right?
>> 
>> Also loading for example a new version freshly compiled of the same file
>> should present the same scenario.
>
> I see a problem in this area.  Consider this code in
> native-elisp-load:
>
>   if (!NILP (Fgethash (filename, all_loaded_comp_units_h, Qnil))
>       && !file_in_eln_sys_dir (filename)
>       && !NILP (Ffile_writable_p (filename)))
>     {
>       /* If in this session there was ever a file loaded with this
> 	 name, rename it before loading, to make sure we always get a
> 	 new handle!  */
>       Lisp_Object tmp_filename =
> 	Fmake_temp_file_internal (filename, Qnil, build_string (".eln.tmp"),
> 				  Qnil);
>       if (NILP (Ffile_writable_p (tmp_filename)))
> 	comp_u->handle = dynlib_open (SSDATA (encoded_filename));
>       else
> 	{
> 	  Frename_file (filename, tmp_filename, Qt);
> 	  comp_u->handle = dynlib_open (SSDATA (ENCODE_FILE (tmp_filename)));
> 	  Frename_file (tmp_filename, filename, Qnil);
> 	}
>
> The last 'else' branch momentarily renames the .eln file, then loads
> it under the modified name, with the assumption that this would force
> dynlib_open to produce a new handle.  But in the case that the same
> .eln file is loaded more than once, i.e. the .eln file was not
> modified since the previous load, dynlib_open returns the same handle
> regardless of the file name, at least on MS-Windows.  (Does this work
> as intended on GNU/Linux?)
>
> The problem with returning the same handle is that the refcount of the
> handle is incremented, which means unload-feature will be unable to
> unload the library.

It works because the handle is stored into the new CU object and passed
to 'load_comp_unit'.  'load_comp_unit' will recognize the "re-load"
condition and discard the CU freshly allocated to use the original one
(that is stored in the .eln).  As a consequence the discarded CU will be
GC'd end so the refcounf will be decremented.

> Is this renaming dance for the case that the .eln file was updated
> since the last load, or do we need it even if it wasn't updated?

The renaming dance is for cases like one changes `comp-speed' recompile
and load.

  Andrea