Segmentation fault from changing constants

[Jutho]

I have been struggling for quite some time to debug a code by one of my students, and finally managed to trace the error down to modifying constant global variables, which are not isbits, and which have been referred to in functions that are compiled. Julia gives a gentle warning upon redefining constants, but you don't expect a segmentation fault resulting from it. The bug was originally in a julia 0.6 code, but the same still happens in 1.1-DEV

const A = randn(10)
function f()
   return sum(A)
end

f() # -> returns something

A = randn(10) # -> redefine, Julia prints a warning

f() # -> still returns the old value

# do some more stuff, until at some point the garbage collector is triggered
GC.gc()

f() # -> segfault, probably not directly, but eventually, just do some more times gc() and other allocating stuff

[Jutho]

I do understand (more or less) why this happens, and why f keeps returning the old value; otherwise I would probably not have figured out the origin of the bug.

But how to prevent other people from running into the same problem? Is it time to be less forgiving in redefining constants in the global scope/REPL?

[KristofferC]

Maybe just make the warning more scary?

[Jutho]

WARNING: redefining constant ... . All hell breaks loose!

[Jutho]

To what extent can we think about constants as just inlined functions that return the constants value? I was wondering wether the same machinery that makes dependence between functions trigger recompilation when a method is redefined can do the same for constants? No idea if that is at all feasible?

[tknopp]

Why is redefining constants allowed at all?

[JeffBezanson]

Julia gives a gentle warning upon redefining constants

The warning is not supposed to be interpreted as gentle! Aside from deprecations, we give warnings quite rarely and only when something is really wrong.

Why is redefining constants allowed at all?

It can be convenient interactively. Maybe it should only be allowed in the REPL?

[tknopp]

I would say that convenient code that segfaults is not that useful.

[Jutho]

The warning is not supposed to be interpreted as gentle! Aside from deprecations, we give warnings quite rarely and only when something is really wrong.

The difference indeed being that using deprecated functions does not lead to segfaults. I thought that Julia did aim at not resulting in segfaults when using pure julia constructions (i.e. no external libraries, over which there is of course no control). In that sense, I guess the warning could be more explicit. Or maybe this problem can be solved as I wrote above, but I am not qualified to tell.

Either way, my main intention was to make to make this unexpected behavior known. The fact that it only appears after a while, when the gc cleans up the original constant, made it very hard to track to down (at least in the more complicated code that I was digging through).

[JeffBezanson]

using deprecated functions does not lead to segfaults

That's why I said "aside from deprecations". But what I don't understand is why somebody would be ok with their program printing warnings. Is the desired outcome really to have the warning, but no segfault? I would think no warning and no segfault would be better. I agree we should probably strengthen the warning though, adding something like this may corrupt your program.

[Jutho]

I think the reason that I interpreted this warning is gentle and typically go over it lightly, is because I read this in the help for the const keyword:

Technically, you can even redefine const variables, although this will generate a warning from the compiler. The only strict requirement is that the type of the variable does not change, which is why const variables are much faster than regular globals.

I am exaggerating a bit here, but I could imagine people interpreting this as: Adding const seems like another trick to let julia do its magic and get a speedup. There is no explicit warning against this in the help.

However I did just find that the docs are more explicit (I don't remember reading that back when I started using Julia, so I should maybe read the latest version again):

Note that although possible, changing the value of a variable that is declared as constant is strongly discouraged. For instance, if a method references a constant and is already compiled before the constant is changed then it might keep using the old value.

Feel free to close.

edited: I don't know why I formatted the word trick as code; now fixed.

[JeffBezanson]

I'll reword the manual and help. The warnings there should be much stronger.

[StefanKarpinski]

Can we just add GC edges from compiled functions to values they depend on? That seems like it would fix this segfault. Or alternately, don't unroot old constant values—i.e. when redefining a constant, put the old value in some sort of semi-permanent root list (we could have an expert API to clear it).

[andyferris]

I’d prefer either to fix it fully (implying #265-style-consistency) by adding backedges to constants and bumping world age when they are modified (I think of methods as like constants...), OR just make the warning an error so you can’t get world “inconsistency” from changing constants at all.

[StefanKarpinski]

People really like reloading files and I don't see why we should make it impossible to do so just because we've made redefining constants an error. Sure, adding 265-style back edges for constants would be good, but rooting all constants seems like it would straightforwardly fix this segfault without causing any major complications. If someone doesn't redefine any constants then it has no impact since their constants are rooted anyway; if someone does redefine constants, then they might end up with some extra rooted values which could potentially leak memory, but that's way better than having segfaults.

[andyferris]

People really like reloading files

No argument there!

So long as we acknowledge the lack of a backedge as a bug, like #265, I'm happy :) (I feel similarly for @pure but let's not go there...)

[StefanKarpinski]

It's not a bug since redefining a constant isn't really something one can expect to work.

[andyferris]

Hmm... sorry, this seems like a curious comment. You're the one that argued people really like to (i.e. expect to be able to) reload code (and potentially redefine constants).

Honestly, I similarly never thought of #265 as a "bug" - it's just the way the system works. I feel this is equivalent to #265 is all (whether we call that a bug or not). Coming from many other programming languages, redifining a method isn't really something one would expect to work, either.

It's just my feeling that the work relating to world consistency is only semi-complete (don't see this as a critism - modern versions of Julia are way better and more usable than earlier versions!). I think you're right that we shouldn't make this an error. and that we should make it work to the best of our ability, and that adding a permanent GC root is a decent solution.

[StefanKarpinski]

My position is this: it's not a bug if redefining a constant doesn't work flawlessly; a segfault, however, is too great a flaw. It's over and above the call of duty for redefinition of a constant to recompile code that depends on it; but of course, sure it would be nice, but not doing it is certainly not a bug.

[JeffBezanson]

The trouble is that not being able to assume global constants are rooted will hinder optimizations. The two categories of problems are (1) needing to set up GC frames and allocate roots in generated code for these values, and (2) the overhead of scanning the extra roots during GC and relocating them when loading the system image.

[StefanKarpinski]

The trouble is that not being able to assume global constants are rooted will hinder optimizations.

My proposal was that old values of constant bindings remain rooted forever (or as long as the module they were once bound in exists), which doesn't seem to undermine that assumption in any way.