[Bug]: out_to_x = false causes spurious error message

[altblue]

[Bug]: out_to_x = false causes spurious error message

[Caellian]

It would also be helpful if the message contained which setting. Given that both set options are bool, I'm assuming there's a third one which defaults to a table type, but isn't expected to with those options.

[Caellian]

I unassigned myself as I'm working on something else at the moment and I'm a little out of my depth with this issue.

This is what I got after looking into this a little:
Message if produced here, because conky.config (userdata) table contains a table key.
That key gets inserted by some property initializer I think, probably out_to_x, but I'm not really familiar with that code so it could be any property that existed when this bug first appeared.

[lineage-of-roots]

@altblue

If seeing that error is the only concern, You could comment out the line

NORM_ERR("invalid setting of type '%s'", l.type_name(type));

currently in src/setting.cc line 55

I have not been able to track down exactly what it is yet, my C++ is too rusty, and my lua is non existent...And there's some gnarly C++ and Lua stuff in there.
I went through it in a debugger for quite a while, but have not been successful in tracking it down yet.

I can confirm that commenting out that line doesn't affect functionality. I was able to get output to ncurses and output to console.
One thing of note is that you have to explicitly specify out_to_x=false. (You're already doing that in your config). Otherwise it assumes it's true if that particular setting is not present.

I don't yet know what this mysterious "setting" is (I am not sure if it is a setting at all).
Could be It's always there once the user defined settings start to get processed and conky "starts" (I noticed the message doesn't appear when out_to_x=true is used. BTW...Both out_to_x and out_to_console can be true at the same time which gets rid of the error message)
Could be a concurrency thing (Might be the conky object itself. Because I noticed the settings don't get processed in the order they appear in the conkyrc file. I put several non existent settings in there to see what would happen. The message appeared but in different order each time)

You get an output to console regardless of how many undefined settings you put in the conkyrc file such as out_to_nowhere=false, out_to_everywhere=false etc etc

That concludes my hobby dive into the conky code for now.

[lineage-of-roots]

@altblue

My updated best guess is that it's running one more time than it(The function getting and setting settings) needs to.
Something about it assuming out_to_x to be true or at least one of the valid out_to_Somewheres and running one more time because of that...Probably.
What I am seeing is that the error message is triggered by a type that is "Table", but it's just null when I do a tocstring() [it's the redefined lua_tolstring()] function) on it.

It would help if my debugger didn't keep crashing, among other weird system stuff.

I have also discovered that if you put any valid settings (such as cpu_avg_samples or net_avg_samples or gap_x, gap_y) in your conkyrc file, you will also get messages saying something like "conky: Invalid value of type 'string' for setting 'gap_x'. Expected value of type 'number'".

These error messages seem from a different era because setting stuff like gap_x and gap_y work just fine, even if it complains about it being the wrong "type".

Since your use case is unique....You can also comment out the following section

if (l.type(index) != Traits::type) {
NORM_ERR(
"Invalid value of type '%s' for setting '%s'. "
"Expected value of type '%s'.",
l.type_name(l.type(index)), Base::name.c_str(),
l.type_name(Traits::type));
return {default_value, false};
}

in the src/setting.hh file

Hope this helps.

[lineage-of-roots]

@Caellian @altblue

I think know what the mysterious setting is....With shaky certainty.
its imlib_cache_size
From what I was able to chop together I got a name out. My knowledge of lua is not enough to be too certain. I haven't figured out "Why" it's still there, just that something is still there in state/stack.

The loop

while (l.next(-2)) {
    l.pop();
    get_setting(l, -1);
  }

in src/setting.cc

goes through the settings in the conkyrc file one more time to tell you about any wrong settings. The actual getting and setting of settings is done elsewhere.

When out_to_x=false is in the conkyrc file, I know with more certainty that the particular function get_setting(l, -1) in this loop is called one more time than it needs to be.
For example if there are 4 settings in there, it gets called 5 times.
If I put out_to_x = false, out_to_console = true, out_to_no_where=true, out_to_every_where = true (the last two being invalid setting names), the function still gets called 5 times.

Even with only one setting in the file, out_to_x = false, it runs twice...But then does say "Unable to find a usable display output"

[lineage-of-roots]

I dug in a little bit more....Turns out it's not imlib_cache_size by itself that's causing the problem. It shows back up in the stack but it may be harmless. However, whenever imlib_cache_size shows up, the mysterious table also shows up on the stack.
This time i was able to peek in it.

It will always have the following key, value pairs in it (When the only setting provided is out_to_x=false in the conkyrc file)

__metatable => (null)
out_to_x => (null)
__index => (null)
music_player_interval => 3.0
own_window => (null)
__newindex => (null)

EDIT: Some of these values are not null. I wasn't converting the booleans
to string. __index & __metatable are Lua specials.

music_player_interval = 3.0 mysteriously shows up in this table. Its similar to imlib_cache_size showing up. However, imlib_cache_size shows up on the stack (Even though it should be gone)
music_player_interval only shows up inside the mystery table (But it shouldn't in there either).

The chunk of code that creates the offending table is (It's not a mystery anymore)

void config_setting_base::make_conky_config(lua::state &l) {
lua::stack_sentry s(l);
l.checkstack(3);

l.newuserdata(1);

l.newtable();
{
l.pushboolean(false);
l.rawsetfield(-2, "__metatable");

l.pushvalue(-1);
l.rawsetfield(-2, "__index");

l.pushfunction(&priv::config_setting_base::config__newindex);
l.rawsetfield(-2, "__newindex");

}
l.setmetatable(-2);

++s;
}
} // namespace priv

in src/setting.cc

Particularly, l.pushvalue(-1) is what's creating a copy of it on the stack.
But I can't mess with it currently because something else breaks somewhere else.
The other mystery is why music_player_interval ends up in there.

I am still left with more questions than answers. How and why are they ending up on the stack where they are.
This behavior is, surprisingly, quite consistent. Which suggests there's some flaw in the logic somewhere. A lot of stuff going on in there (over my head) because I barely know anything about Lua.
I can't get a garbage collector thing or memory leak because it's consistent.

This turned out to be alot more complicated than I thought... for me at least.

---

---

And now for a slight tangent (Which I could be wrong about)

This chunk of code that calls get_setting()

void config_setting_base::process_setting(lua::state &l, bool init) {
lua::stack_sentry s(l, -3);

config_setting_base *ptr = get_setting(l, -3);
if (ptr == nullptr) { return; }

from here, I don't think get_setting() does much of anything. I don't see how it could throw its "invalid setting of type" or "unknown setting" error. It's index -3, there's no tables, and the names are set correctly before hand.
If there are any errors in the conkyrc file, conky will crash long before it gets here.

It only does something when get_setting() is called from the

while (l.next(-2)) {
l.pop();
get_setting(l, -1);
}

loop with index -1

I checked with wrong setting names, they only trigger when called from this while loop(Where the mysterious table also shows up on the stack).

The wrong setting "names" trigger here but actual syntax errors or tables in the conkyrc file make conky crash way before here.
I think the table type check was put in there to make conky not crash even with valid settings.

const std::string &name = l.tostring(index);
will crash with a table

The table only shows up through this while loop.
I don't yet understand what l.next(-2) is doing, but it does seem to change the stack.

[Caellian]

Added some debugging to the loop that checks for invalid setting names:

1 conky: value is 'boolean'
1 conky: key is 'out_to_console'
2 conky: value is 'boolean'
2 conky: key is 'out_to_x'
3 conky: value is 'imlib_cache_size'
3 conky: key is 'table'
> conky: invalid setting of type 'table'

It looks like before imlib_cache_size there's an extra push/pop.
Tried moving imlib_cache_size global static into conky-imlib2.cc which made it load first, so at the first location same thing happened.

That tells me that the error happens in the first pass, only imlib_cache_size is not having nil on top of stack both before and after calling lua_setter.

EDIT:

Found the bug, it's in imlib_cache_size_setting::lua_setter.

There's an early return if display == nullptr || window.visual == nullptr which doesn't increment stack sentry (++s).

@lineage-of-roots thanks for your help.

[lineage-of-roots]

@Caellian

Fist thing is that I now can't resist making a pun......

Ghost in the Stack::Lua *State Society

or

Ghost in the Stack -> Lua *State C-osiety

And nice! Glad I helped. Finally solved!
I went through conky-imlib2.cc before, but I missed it.
I am, however, still thinking about how music_player_interval showed up in the mystery table.

---

---

I have a proposal as well.....

I think process_setting() can look like this.

void config_setting_base::process_setting(lua::state &l, bool init) {
  lua::stack_sentry s(l, -3);

   const std::string &name = l.tostring(-3);
  auto iter = settings->find(name);
  config_setting_base *ptr = iter->second;

........
.......
....

No need to call get_setting() in process_setting()

Whereas get_setting() can look like this

priv::config_setting_base *get_setting(lua::state &l, int index) 
{
 
  const std::string &name = l.tostring(index);
  auto iter = settings->find(name);
  if (iter == settings->end()) {
    NORM_ERR("Unknown setting name '%s'", name.c_str());
       }
   return 0;
  }

Also, perhaps rename get_setting() to get_invalid_settings()

[Caellian]

Ghost in the Stack -> Lua *State C-osiety

Lol, I didn't get that pun before asking an LLM to explain it to me. That's funny.

I have a proposal as well.....
[... proposal ...]

You can create another issue if you want or (better yet 😏) submit a PR.

There's a whole bunch of bugs that are more pressing, and a whole lot of features I'd personally like to add to conky before improving Lua stack related code - it takes up a lot of time to test it and it's very annoying to debug when it doesn't work. If it aint broke I'll gladly leave it as it is.

[lineage-of-roots]

@Caellian

You have to show me the LLM output about the pun.
I'm interested to know how it explained it.

And as far as git goes.....Well...I don't yet know enough about git to do any pulling or pushing lol. It would just be a hindrance if I tried it now.

I get the sentiment about not wanting to change unbroken stuff. It was annoying to debug, so we'll leave it for now.
I did however check beforehand if get_setting is called from anywhere else. It's only ever called from within this file and nowhere else.

In other news...I find out where music_player_interval was coming from

void music_player_interval_setting::lua_setter(lua::state &l, bool init) {
  lua::stack_sentry s(l, -2);

  if (l.isnil(-2)) {
    l.checkstack(1);
    l.pushnumber(update_interval.get(l));
    l.replace(-3);
  }

  Base::lua_setter(l, init);

  ++s;
}

If it's not provided, it gets set up anyway.

It shows up in the (previously mysterious) table on the stack.
Other values always in that table will always be "out_to_x" and "own_window" (alongside any other settings in the conkyrc file)

The line

  l.rawset(-4);

in the function process_setting is what throws it(and others) in the previously mysterious table.

I am guessing they are left in there because somewhere down the line mpd, or cmus or audacious might throw a fit in the code if music_player_interval isn't there.

Same goes for own_window and out_to_x...stuff might break without them perhaps.

---

---

Some extra info for the sake of posterity

There will be 3 items on the stack before everything gets popped off and the main_loop() begins
All three are tables

One of them will contain the conky.text from the conkyrc file
It will look something like this

config 
text = 
${whatever was in provided in the conkyrc file}


${whatever else was provided in the conkyrc file} and so on

variables 
astext 
asnumber 

One table will contain ONLY and ONLY whatever settings were provided in the conkyrc file

The last one will contain whatever settings were provided in the conkyrc file along with the following that will always be there.

music_player_interval = 3.0
__metatable 
__newindex 
out_to_x 
__index 
own_window 

[Caellian]

You have to show me the LLM output about the pun.
I'm interested to know how it explained it.

I am guessing they are left in there because somewhere down the line mpd, or cmus or audacious might throw a fit in the code if music_player_interval isn't there.

Yeah, some settings use other settings in their lua_setter. This is something I'd like to change at some point as well because it makes state management very complicated and encodes loading order in the sources:

• Ideally, settings should be returning std::optional by default if they're not set by users.
• Settings shouldn't need a special load order list as it's hard to maintain and error prone.

I think that can be handled by making some changes to lua_setter, but I'll investigate later.

[lineage-of-roots]

That LLM needs more internet access.
It missed the other half of the pun.

I was also referencing Ghost in the Shell: Stand Alone Complex - Solid State Society
It's a movie.
I am guessing you haven't seen Ghost in the Shell 1997 or the Anime series?
I highly recommend the Movies and the Series. The old ones.

---

Now back to business

I even went looking at things like Pluto and Eris to see if I could dump the whole state to see all the stuff in there.

My method was brute force, i.e. many many stack dumps after many many stack operations till I narrowed it down. It was quite annoying.

I also realized that (one peculiar instance of) out_to_x was being set even before load_file was called. It happens here

#ifdef BUILD_WAYLAND
  state->pushboolean(true);
  out_to_wayland.lua_set(*state);
#else
#ifdef BUILD_X11
  state->pushboolean(true);
  out_to_x.lua_set(*state);
#else
  state->pushboolean(true);
  out_to_stdout.lua_set(*state);
#endif
#endif

That out_to_x.lua_set() up there puts out_to_x = true into a table that's never seen again on the stack. I forgot to check what happens if I build both Wayland and X11.

Also things called on the stack not having a particular order really throws you for a loop. (Such as the settings provided in the conkyrc file will have no particular order in which they will show up on the stack). Some times it would mess up on the first time, other times it would mess up on the second or third loop or more (depending on how many settings were in there).
I learned that this is just how Lua works, so can't be helped.

I checked the release source code from 2015, get_setting() has had that table type check since 2015.
Which suggests something like this had happened back then as well.
imlib_cache_size_setting::lua_setter() was okay though, ++s was not skipped.

I don't yet know how to make git search for commits on a single file to see when it changed. I would like to know when imlib_cache_size code was changed.