Odd Lua Issue in 0.6.2

[kripken]

Running the following code,

Code: Select all

print( lastfocus:getName() )
letfocus:activate()


I get correct output for the print command (the name of the window), but a crash on the next line,

Code: Select all

CEGUI::ScriptException in file
...CEGUILuaFunctor.cpp(195): Unable to call Lua event handler:

attempt to call a nil value


What makes this odd is that this is on Windows, with 0.6.2, while (1) this code worked fine with 0.6.1 on Windows, and (2) this code works fine on Linux with 0.6.2.

I'm not even sure what the problem could be, as the print command proves that lastfocus is a valid window, and if activate were an invalid function name then the error message would be different, in particular, it would be informative, giving the line number in Lua of the error, unlike here - I had to find the crashing line by placing lots of debug messages and seeing where it stopped.

Any ideas on how to get to the bottom of this?

[CrazyEddie]

HI,

I assume that 'letfocus' in the second line is just a typo in the forum post?

Aside from this, did you also clean / rebuild your windows binaries against 0.6.2 - I'm guessing you probably did after the ABI issue that came up on linux.

What does the original subscription line that bound this look like?

CE.

[kripken]

HI,

I assume that 'letfocus' in the second line is just a typo in the forum post?

Aside from this, did you also clean / rebuild your windows binaries against 0.6.2 - I'm guessing you probably did after the ABI issue that came up on linux.

What does the original subscription line that bound this look like?

CE.

Yeah, 'letfocus' was a typo here in the forum post, not the original source.

I did a complete rebuild (twice, even) from a new directory, to make sure there wasn't any confusion with the old version. Same problem, entirely consistent.

This isn't in an event handler, so there isn't a subscription call that is relevant. This is from code that sets up the window layout for my login screen. The crash is when it tries to do activate() to the 'username' field, which I want to be active.

If you want, you can see the source code here, line 186 is where it crashes (it's called from Login.lua). Of course this is a lot to read, and I have no idea what part of it - if this is even caused by the Lua code, which as I said works in Linux - might be relevant to this crash.

I'm using the prebuilt VC2008 binaries for CEGUI 0.6.2. Were they built differently somehow from the 0.6.1 binaries?

[CrazyEddie]

This isn't in an event handler, so there isn't a subscription call that is relevant. This is from code that sets up the window layout for my login screen. The crash is when it tries to do activate() to the 'username' field, which I want to be active.

Hmmm. I asked about that since the error made mention of the CEGUILuaFunctor.cpp file - this is typically only used for despatching event subscriptions made from within lua.

It's really odd that it works in linux. Do you get the same result from debug and release builds on Windows?

I can't help but feel I somehow dropped the ball as far as the lua module in the new release goes I'm somewhat tempted to compile this so I can reproduce the issue locally and maybe work out what's going on, though this wouldn't be for maybe a day or three.

I'm using the prebuilt VC2008 binaries for CEGUI 0.6.2. Were they built differently somehow from the 0.6.1 binaries?

I think they were built in the same manner with the same configuration, scriptkid would be able to confirm this for sure, since he built them

CE

[kripken]

It's really odd that it works in linux. Do you get the same result from debug and release builds on Windows?

Ok, I spent another half-day on this now. Yes, the results are the same with both debug and release builds, and indeed it runs fine on Linux but crashes on Windows.

I also did the following test on Windows, a refinement of what I did earlier: The exact same code, only replacing getChild with getChildRecursive, works perfectly with 0.6.1, but crashes with 0.6.2.

Here is the actual error:

Code: Select all

09/12/2008 11:15:55 (Error)   CEGUI::ScriptException in file d:\temp\cegui-0.6.2-vc9\scriptingmodules\ceguilua\luascriptmodule\src\ceguiluafunctor.cpp(195) : Unable to call Lua event handler:



attempt to call a nil value




(d:\temp is from how the 0.6.2 binaries were built, I presume, as it isn't anything on my system) This occurs from an error in a Lua script that has been called from executeScript, not an event handler (I do executeScript to set up the layout).

The crashing behaviour is erratic, i.e., the code is fragile - move the calls around, and the crash occurs elsewhere. Crashes can occur during startup or in response to a keypress. I spent a while trying to find some pattern here, but it seems there isn't - so my guess is that there might be some memory corruption that would lead to this sort of fragility.

I would try to get more to the bottom of this, but I'm not that proficient on Windows I'm afraid (Linux is my area) - I don't even know how to run my app in the VC++ debugger, given that it's actually used as a Python module, not a normal exe or dll.

In the meantime I'll revert my project back to 0.6.1 as it works fine with that.

[CrazyEddie]

Hi,

Thanks for testing the various configurations. I only wish I could come here and say "Ah, yes. Do A..B..C.. and it'll fix it", but alas, at present I have no insights to offer.

(d:\temp is from how the 0.6.2 binaries were built, I presume, as it isn't anything on my system)

This is correct.

The crashing behaviour is erratic, i.e., the code is fragile - move the calls around, and the crash occurs elsewhere. Crashes can occur during startup or in response to a keypress. I spent a while trying to find some pattern here, but it seems there isn't

Yeah, these types of issue are really fun to track down (not).

so my guess is that there might be some memory corruption that would lead to this sort of fragility.

While I agree with this, it's really odd that it's a Windows only issue.

I would try to get more to the bottom of this, but I'm not that proficient on Windows I'm afraid (Linux is my area)

I prefer to spend my time away from Windows also, though I am somewhat skilled in using the debugger there. Tomorrow I'll start getting the required things to build the engine and attempt to reproduce and debug the issue (mainly because it seems to be a CEGUI issue, I'm not really in the habit of debugging other projects' code ).

In the meantime I'll revert my project back to 0.6.1 as it works fine with that.

Yeah, at least you're not left totally high-and-dry by this. Though it is a real pain, since 0.6.2 was supposed to fix issues, not make them

CE.

[kripken]

Tomorrow I'll start getting the required things to build the engine and attempt to reproduce and debug the issue (mainly because it seems to be a CEGUI issue, I'm not really in the habit of debugging other projects' code ).

Understood. Thanks for doing this!

Hopefully building the engine will be easy, there are instructions on the download page. Note in particular the 'complete build environment' download, which should save you getting all the various libraries, header files, etc. (including CEGUI 0.6.1 )

More importantly, I am on IRC (both on #cegui and #intensityengine) most of the day, so we can discuss building and debugging this issue there, if that's convenient for you.

[Pompei2]

Maybe the memory issue (if it is one) is caused by some class members being left uninitialised ? This often happens to me as gcc seems to fill them with 0's by default whereas visual c++ gives them (random) values.

Maybe it is also possible that you have subscribed to the "window activated" event somewhere bit having a wrong function pointer or so?

Just a few ideas.

[CrazyEddie]

@kripken: Ok, I had some time this evening, so have downloaded everything, built the thing, run it successfully, updated to CEGUI 0.6.2, rebuilt, and after updating some getChild calls to getChildRecursive (hehe ) I am able to get the same error I'll leave the debugging of it for tomorrow when I'm fresher.

@Pompei2: Yeah, the uninitialised member is something I was pouring over the source for yesterday There's actually a couple of rare cases where this could happen too, though I do not believe that it's causing this (find out tomorrow). So it's either something like that, or perhaps somewhere the lua stack is getting messed up.

CE.

[kripken]

@kripken: Ok, I had some time this evening, so have downloaded everything, built the thing, run it successfully, updated to CEGUI 0.6.2, rebuilt, and after updating some getChild calls to getChildRecursive (hehe ) I am able to get the same error I'll leave the debugging of it for tomorrow when I'm fresher.

Ok, I'm glad you have the same error, well I mean I'm glad that at least it isn't all in my head

Maybe the memory issue (if it is one) is caused by some class members being left uninitialised ? This often happens to me as gcc seems to fill them with 0's by default whereas visual c++ gives them (random) values.

That's interesting, I should probably learn more about such different compiler behaviours.

[CrazyEddie]

Ok, as mentioned on IRC, this turned out to be an issue where the lua function references were getting released by accident when some temporary objects got destroyed.

The fix is in branches/v0-6 at rev. 1895. I'm also posting the patch below:

Code: Select all

Index: ScriptingModules/CEGUILua/LuaScriptModule/include/CEGUILuaFunctor.h
===================================================================
--- ScriptingModules/CEGUILua/LuaScriptModule/include/CEGUILuaFunctor.h   (revision 1894)
+++ ScriptingModules/CEGUILua/LuaScriptModule/include/CEGUILuaFunctor.h   (working copy)
@@ -87,6 +87,14 @@
     static void pushNamedFunction(lua_State* L, const String& name);
 
 private:
+    /*!
+    \brief
+        Invalidate the registry references.  This is used internally to ensure
+        that the references do not get released (for example when we destroy
+        a temporary object)
+    */
+    void invalidateLuaRefs();
+
     lua_State* L;
     mutable int index;
     int self;
Index: ScriptingModules/CEGUILua/LuaScriptModule/src/CEGUILua.cpp
===================================================================
--- ScriptingModules/CEGUILua/LuaScriptModule/src/CEGUILua.cpp   (revision 1894)
+++ ScriptingModules/CEGUILua/LuaScriptModule/src/CEGUILua.cpp   (working copy)
@@ -232,18 +232,21 @@
     const String& err_str = getActivePCallErrorHandlerString();
     const int err_ref     = getActivePCallErrorHandlerReference();
 
+    Event::Connection con;
     // do the real subscription
-    LuaFunctor functor((err_ref == LUA_NOREF) ?
-                LuaFunctor(d_state, subscriber_name, LUA_NOREF, err_str) :
-                LuaFunctor(d_state, subscriber_name, LUA_NOREF, err_ref));
-    Event::Connection con =
-        target->subscribeEvent(event_name, Event::Subscriber(functor));
+    if (err_ref == LUA_NOREF)
+    {
+        LuaFunctor functor(d_state, subscriber_name, LUA_NOREF, err_str);
+        con = target->subscribeEvent(event_name, Event::Subscriber(functor));
+        functor.invalidateLuaRefs();
+    }
+    else
+    {
+        LuaFunctor functor(d_state, subscriber_name, LUA_NOREF, err_ref);
+        con = target->subscribeEvent(event_name, Event::Subscriber(functor));
+        functor.invalidateLuaRefs();
+    }
 
-    // make sure we don't release the reference we just made when 'functor' is
-    // destroyed as it goes out of scope.
-    functor.index = LUA_NOREF;
-    functor.d_errFuncIndex = LUA_NOREF;
-
     // return the event connection
     return con;
 }
@@ -258,18 +261,23 @@
     const String& err_str = getActivePCallErrorHandlerString();
     const int err_ref     = getActivePCallErrorHandlerReference();
 
+    Event::Connection con;
     // do the real subscription
-    LuaFunctor functor((err_ref == LUA_NOREF) ?
-                LuaFunctor(d_state, subscriber_name, LUA_NOREF, err_str) :
-                LuaFunctor(d_state, subscriber_name, LUA_NOREF, err_ref));
-    Event::Connection con =
-        target->subscribeEvent(event_name, group, Event::Subscriber(functor));
+    if (err_ref == LUA_NOREF)
+    {
+        LuaFunctor functor(d_state, subscriber_name, LUA_NOREF, err_str);
+        con = target->subscribeEvent(event_name, group,
+                                     Event::Subscriber(functor));
+        functor.invalidateLuaRefs();
+    }
+    else
+    {
+        LuaFunctor functor(d_state, subscriber_name, LUA_NOREF, err_ref);
+        con = target->subscribeEvent(event_name, group,
+                                     Event::Subscriber(functor));
+        functor.invalidateLuaRefs();
+    }
 
-    // make sure we don't release the reference we just made when 'functor' is
-    // destroyed as it goes out of scope.
-    functor.index = LUA_NOREF;
-    functor.d_errFuncIndex = LUA_NOREF;
-
     // return the event connection
     return con;
 }
Index: ScriptingModules/CEGUILua/LuaScriptModule/src/CEGUILuaFunctor.cpp
===================================================================
--- ScriptingModules/CEGUILua/LuaScriptModule/src/CEGUILuaFunctor.cpp   (revision 1894)
+++ ScriptingModules/CEGUILua/LuaScriptModule/src/CEGUILuaFunctor.cpp   (working copy)
@@ -54,6 +54,7 @@
     index(func),
     self(selfIndex),
     needs_lookup(false),
+    d_errFuncIndex(LUA_NOREF),
     d_ourErrFuncIndex(false)
 {
     // TODO: This would perhaps be better done another way, to avoid the
@@ -77,6 +78,7 @@
     self(selfIndex),
     needs_lookup(true),
     function_name(func),
+    d_errFuncIndex(LUA_NOREF),
     d_ourErrFuncIndex(false)
 {
     // TODO: This would perhaps be better done another way, to avoid the
@@ -384,32 +386,53 @@
         // reference function
         int index = luaL_ref(L, LUA_REGISTRYINDEX);
 
-        LuaFunctor functor((err_idx != LUA_NOREF) ?
-                        LuaFunctor(L, index, thisIndex, err_idx) :
-                        (!err_str.empty()) ?
-                            LuaFunctor(L, index, thisIndex, err_str) :
-                            LuaFunctor(L, index, thisIndex));
-        con = self->subscribeEvent(String(event_name), Event::Subscriber(functor));
-        // make sure we don't release the reference(s) we just made when the
-        // 'functor' object is destroyed (goes out of scope)
-        functor.index = LUA_NOREF;
-        functor.self = LUA_NOREF;
-        functor.d_errFuncIndex = LUA_NOREF;
+        if (err_idx != LUA_NOREF)
+        {
+            LuaFunctor functor(L, index, thisIndex, err_idx);
+            con = self->subscribeEvent(String(event_name),
+                                       Event::Subscriber(functor));
+            functor.invalidateLuaRefs();
+        }
+        else if (!err_str.empty())
+        {
+            LuaFunctor functor(L, index, thisIndex, err_str);
+            con = self->subscribeEvent(String(event_name),
+                                       Event::Subscriber(functor));
+            functor.invalidateLuaRefs();
+        }
+        else
+        {
+            LuaFunctor functor(L, index, thisIndex);
+            con = self->subscribeEvent(String(event_name),
+                                       Event::Subscriber(functor));
+            functor.invalidateLuaRefs();
+        }
     }
     else if (type == LUA_TSTRING)
     {
         const char* str = lua_tostring(L, -1);
-        LuaFunctor functor((err_idx != LUA_NOREF) ?
-                        LuaFunctor(L, String(str), thisIndex, err_idx) :
-                        (!err_str.empty()) ?
-                            LuaFunctor(L, String(str), thisIndex, err_str) :
-                            LuaFunctor(L, String(str), thisIndex));
 
-        con = self->subscribeEvent(String(event_name), Event::Subscriber(functor));
-        // make sure we don't release the reference(s) we just made when the
-        // 'functor' object is destroyed (goes out of scope)
-        functor.self = LUA_NOREF;
-        functor.d_errFuncIndex = LUA_NOREF;
+        if (err_idx != LUA_NOREF)
+        {
+            LuaFunctor functor(L, String(str), thisIndex, err_idx);
+            con = self->subscribeEvent(String(event_name),
+                                       Event::Subscriber(functor));
+            functor.invalidateLuaRefs();
+        }
+        else if (!err_str.empty())
+        {
+            LuaFunctor functor(L, String(str), thisIndex, err_str);
+            con = self->subscribeEvent(String(event_name),
+                                       Event::Subscriber(functor));
+            functor.invalidateLuaRefs();
+        }
+        else
+        {
+            LuaFunctor functor(L, String(str), thisIndex);
+            con = self->subscribeEvent(String(event_name),
+                                       Event::Subscriber(functor));
+            functor.invalidateLuaRefs();
+        }
     }
     else
     {
@@ -422,5 +445,13 @@
 }
 
 //----------------------------------------------------------------------------//
+void LuaFunctor::invalidateLuaRefs()
+{
+    index = LUA_NOREF;
+    self = LUA_NOREF;
+    d_errFuncIndex = LUA_NOREF;
+}
 
+//----------------------------------------------------------------------------//
+
 } // namespace CEGUI

CE.

[CrazyEddie]

We are pleased to be able to offer CEGUI 0.6.2b - which is a re-issue of the original 0.6.2 which is now being withdrawn - that addresses this problem. See the announcement here.

HTH

CE

[Pompei2]

uh oh

[CrazyEddie]

uh oh

Just to cross-reference a couple of things properly. I think this issue is almost certainly the same issue as originally reported here.

CE.

[kripken]

We are pleased to be able to offer CEGUI 0.6.2b - which is a re-issue of the original 0.6.2 which is now being withdrawn - that addresses this problem. See the announcement here.

HTH

CE

Thanks for the quick release of 0.6.2b. I compiled my project on Windows with it and all is well