GNU bug report logs - #3892
corrupting load-in-progress value with "let"

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 3892 in the body.
You can then email your comments to 3892 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Ken Raeburn <raeburn <at> raeburn.org>
To: bug-gnu-emacs <at> gnu.org
Subject: corrupting load-in-progress value with "let"
Date: Tue, 21 Jul 2009 00:01:12 -0400

With these source files:

==> test.el <==
(message "test:  load-in-progress %S" load-in-progress)
(let ((load-path (cons "." load-path)))
  (load "test2"))
(message "test:  load-in-progress %S" load-in-progress)

==> test2.el <==
(message "test2: load-in-progress %S" load-in-progress)
(load "test3")
(message "test2: load-in-progress %S" load-in-progress)

==> test3.el <==
(message "test3: load-in-progress %S" load-in-progress)

and test.el and test2.el byte-compiled but test3.el only present in  
source form, the resulting output shows that load-in-progress is  
inconsistent:

% ./b/Inst/bin/emacs --batch -l test.elc
test:  load-in-progress t
Loading test2...
test2: load-in-progress t
Loading /tmp/emacs-23.0.96/test3.el (source)...
test3: load-in-progress t
test2: load-in-progress t
test:  load-in-progress nil
%

I'm working with the CVS sources and 23.0.96 prerelease code, but can  
reproduce the behavior in 22.1 as shipped with Mac OS X.

The variable load-in-progress is defined in the C code with  
DEFVAR_BOOL applied to an int variable.  Since file loading can be  
invoked recursively, the int value is incremented and decremented when  
loading begins and ends, and should in theory be zero only when no  
file is being loaded.

However, if we're loading a .el file from source, the C code in  
lread.c:Fload calls out to the load-source-file-function, which winds  
up calling code in mule.el, which uses "let" on load-in-progress.  The  
let/unwind support doesn't save and restore the integer value for a  
Lisp_Misc_Boolfwd variable, just the boolean state.  So after loading  
of test3.el finishes above, load-in-progress is restored from its old  
*boolean* value, and gets the value 1 instead of 2 as it should have.   
When test2.elc is done loading, it drops to zero and it looks like  
we're not currently loading any files, even though we're in the middle  
of loading test.elc still.

There is code (in C mode, among others) which checks whether a file is  
being loaded, so this can have a behavioral impact under certain  
conditions.  I haven't actually triggered the problem in my normal  
usage patterns though.

On the assumption that DEFVAR_BOOL variables really ought to just be  
used as booleans (I haven't checked other boolean variables though),  
and we don't want to change the Lisp-visible binding to an integer (or  
"if load-in-progress" would stop working right), I'm working on a  
patch to make load-in-progress an actual boolean, and put the file- 
loading depth counter elsewhere, inaccessible to Lisp (since it's  
inaccessible now).

Ken

Message #10 received at 3892 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Ken Raeburn <raeburn <at> raeburn.org>
Cc: 3892 <at> debbugs.gnu.org
Subject: Re: bug#3892: corrupting load-in-progress value with "let"
Date: Tue, 21 Jul 2009 12:27:05 -0400

> The variable load-in-progress is defined in the C code with DEFVAR_BOOL
> applied to an int variable.  Since file loading can be  invoked recursively,
> the int value is incremented and decremented when  loading begins and ends,
> and should in theory be zero only when no  file is being loaded.

> However, if we're loading a .el file from source, the C code in
> lread.c:Fload calls out to the load-source-file-function, which winds  up
> calling code in mule.el, which uses "let" on load-in-progress.
> The  let/unwind support doesn't save and restore the integer value for
> a  Lisp_Misc_Boolfwd variable, just the boolean state.  So after loading  of
> test3.el finishes above, load-in-progress is restored from its old
> *boolean* value, and gets the value 1 instead of 2 as it should have.
> When test2.elc is done loading, it drops to zero and it looks like  we're
> not currently loading any files, even though we're in the middle  of loading
> test.elc still.

Good catch.

> On the assumption that DEFVAR_BOOL variables really ought to just be used as
> booleans (I haven't checked other boolean variables though),  and we don't
> want to change the Lisp-visible binding to an integer (or "if
> load-in-progress" would stop working right), I'm working on a  patch to make
> load-in-progress an actual boolean, and put the file-
> loading depth counter elsewhere, inaccessible to Lisp (since it's
> inaccessible now).

I think we should drop the counter altogether, and use specbind instead.


        Stefan

Message #15 received at 3892 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Ken Raeburn <raeburn <at> raeburn.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 3892 <at> debbugs.gnu.org
Subject: Re: bug#3892: corrupting load-in-progress value with "let"
Date: Tue, 21 Jul 2009 14:44:42 -0400

On Jul 21, 2009, at 12:27, Stefan Monnier wrote:
> I think we should drop the counter altogether, and use specbind  
> instead.

Much simpler, yes, and it looks like nothing in the C code cares about  
the numeric value either.
Thanks for the suggestion; I'll revise my patch.

Ken

Message #20 received at 3892 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Ken Raeburn <raeburn <at> raeburn.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 3892 <at> debbugs.gnu.org
Subject: Re: bug#3892: corrupting load-in-progress value with "let"
Date: Thu, 23 Jul 2009 20:08:39 -0400

On Jul 21, 2009, at 14:44, Ken Raeburn wrote:
> On Jul 21, 2009, at 12:27, Stefan Monnier wrote:
>> I think we should drop the counter altogether, and use specbind  
>> instead.
>
> Much simpler, yes, and it looks like nothing in the C code cares  
> about the numeric value either.
> Thanks for the suggestion; I'll revise my patch.

I'm working on the revised patch, but it occurs to me that this way,  
some broken bit of code could set load-in-progress to t at some point  
when nothing is being loaded, and it will never become nil again  
unless explicitly reset.  I'm not sure that's right either.  I think I  
kind of like the behavior that while code can temporarily override it,  
the correct state will be restored from the C code any time it changes  
(file loading starts or completes).

A related issue: If we're changing the way it's set, should we retain  
the constraint that load-in-progress can only (appear to) hold boolean  
values, or let it hold any Lisp value instead?

Ken

Message #25 received at 3892 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Ken Raeburn <raeburn <at> raeburn.org>
Cc: 3892 <at> debbugs.gnu.org
Subject: Re: bug#3892: corrupting load-in-progress value with "let"
Date: Thu, 23 Jul 2009 21:14:48 -0400

>>> I think we should drop the counter altogether, and use specbind instead.
>> Much simpler, yes, and it looks like nothing in the C code cares about the
>> numeric value either.
>> Thanks for the suggestion; I'll revise my patch.

> I'm working on the revised patch, but it occurs to me that this way, some
> broken bit of code could set load-in-progress to t at some point  when
> nothing is being loaded, and it will never become nil again  unless
> explicitly reset.

Isn't that a problem that already exists?
In any case such setting would be a bug, so I wouldn't worry about it.

> A related issue: If we're changing the way it's set, should we retain the
> constraint that load-in-progress can only (appear to) hold boolean  values,
> or let it hold any Lisp value instead?

A boolean seems sufficient, I see no need to change it for now,


        Stefan

Message #30 received at 3892 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Ken Raeburn <raeburn <at> raeburn.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 3892 <at> debbugs.gnu.org
Subject: Re: bug#3892: corrupting load-in-progress value with "let"
Date: Thu, 23 Jul 2009 21:51:01 -0400

On Jul 23, 2009, at 21:14, Stefan Monnier wrote:
>> I'm working on the revised patch, but it occurs to me that this  
>> way, some
>> broken bit of code could set load-in-progress to t at some point   
>> when
>> nothing is being loaded, and it will never become nil again  unless
>> explicitly reset.
>
> Isn't that a problem that already exists?

Not exactly... in the released code (and trunk before my first  
change), it can explicitly be set to t at the top level, but it'll be  
reset to nil after loading another file.  And, the bug I was aiming to  
fix initially, the counter (unseen by Lisp) can be set to 1 from some  
higher number during nested loading, causing it to be set to nil when  
returning back to enclosing load calls, but once you get back to the  
top level the nil value is correct, and when the next load starts from  
the top level it'll correctly be set to t (temporarily) again.

So the specbind version would make the first bug a little worse,  
though it fixes the second.

> In any case such setting would be a bug, so I wouldn't worry about it.

Yeah, that's true enough...

>> A related issue: If we're changing the way it's set, should we  
>> retain the
>> constraint that load-in-progress can only (appear to) hold boolean   
>> values,
>> or let it hold any Lisp value instead?
>
> A boolean seems sufficient, I see no need to change it for now,

Okay, it doesn't seem like a big deal to me either way.

Ken

Message #35 received at 3892-done <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Chong Yidong <cyd <at> stupidchicken.com>
To: 3892-done <at> debbugs.gnu.org
Subject: Re: bug#3892: corrupting load-in-progress value with "let"
Date: Sat, 15 Aug 2009 19:10:34 -0400

Closing the bug (see 2009-07-25 checkin to trunk by Ken Raeburn).

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.