Sway: Problems with git master versions of json-c

Created on 13 Sep 2017 · 16Comments · Source: swaywm/sway

Short version: It won't compile out of the box. If do you change the code in order to compile it, it will crash whenever you use any key binding. I eventually decided to see if there were earlier tags of json-c that wouldn't have this problem and it seems that even with the latest one, version 0.12.1, sway compiles and runs just fine. Maybe the maximum version of json-c with which sway will work should be indicated in README.

Long version:
When compiling, you get type mismatch errors in five files, all involving json_object_array_length in for loops:
sway/ipc-server.c: In function ‘ipc_client_handle_command’: sway/ipc-server.c:449:21: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare] for (int i = 0; i < json_object_array_length(request); i++) {

The five files are:

sway/ipc-server.c
swaybar/status_line.c
swaybar/tray/tray.c
swaygrab/json.c
swaylock/main.c

The solution for that is very simple. Merely change the type of the counter variables from int to size_t.

On pressing keys, I've traced the problem to the line in ipc-server.c:

json_object_object_add(obj, "binding", sb_obj);

My guess is that the call to json_object_put(obj); later in the function frees the objects recursively and then sb_obj is used (or more likely freed, as the stdout error I get from glibc is *** Error in 'sway': corrupted double-linked list:) later, causing an Earth-shattering kaboom.

Here's the log file. I used Lynx because I couldn't figure out how to upload a file with my usual browser (though, I know, Lynx should be be usual browser; with Lynx, all I had to do was ^Xe:e ~/.sway.log<Return>VGy:e #1<Return>p:wq) and I seem not to have been logged into to github, so it's anonymous gist. Hmm.
https://gist.github.com/anonymous/fe18bb44c7d40d5d63fbf15427e70fdc

Source

keithbowes

Most helpful comment

@cauebs Use sway v0.15.1 release, which has all needed fixes.

besser82 on 3 Jan 2018

👍2

All 16 comments

Looks like you have enough information to put a patch together?

ddevault on 13 Sep 2017

I got it to compile by changing the CmakeLists.txt file to ignore warnings, but I have no idea how to fix the crash. Mere typecasting doesn't work. I guess you'd have to really change a lot of Sway's internal code so that it uses the same data types as json-c.

keithbowes on 25 Oct 2017

Hi @keithbowes, I took a look at your issue, and I think besides explicitly requiring the latest json-c version in the cmake file there's really nothing that can be done.

When they release the next version, which will probably have this API break, we can update the code by converting the counters to size_t.

I'm not sure if @SirCmpwn would agree to this.

mariusor on 25 Oct 2017

Oh, how I love C++11 auto <3

ranisalt on 26 Oct 2017

This issue just hit Fedora Rawhide (future Fedora 28). As json-c 0.13 has been released now, are there any plans to adapt sway accordingly? We can't enforce json-c <= 0.12 in Fedora.

See https://bugzilla.redhat.com/show_bug.cgi?id=1525020 for the Fedora-related discussion.

morxa on 13 Dec 2017

FYI: Looks like this was actually a json-c bug and should be fixed by json-c/json-c#389. Not exactly sure if this is the same issue, but it certainly looks like it.

morxa on 14 Dec 2017

I'll be getting a bugfix release out fairly soon, and will address this in it.

ddevault on 14 Dec 2017

Wait, if it's a bug with json-c, can Fedora ship a patched version?

ddevault on 14 Dec 2017

Yes, Fedora now includes the json-c patch in Rawhide (commit), so at least from Fedora side, the problem is fixed. I would suggest to test the patched version of json-c before doing any other work related to this issue.

morxa on 14 Dec 2017

The problem was in Sway actually… The current patch Fedora ships was a cludge to restore the faulty behaviour when double free'ing a json_object… (see: https://github.com/swaywm/sway/pull/1517/commits/f325e5e2c54228682c2fe919c5d143b16139194f)

The fixes needed for Sway are in #1517.

Just to clarify this a bit more: The real problem has been in Sway since https://github.com/swaywm/sway/commit/6392abe35b32c66c642c25a7c6911021862d3413. There was a bug in json_object_put() (which is the dtor function with ref-counting for a json_object) in all versions of json-c prior to v0.13, which camouflaged any invalid call of the dtor with an already destroyed json_object.

I will apply this set of patches to Sway in Fedora and update json-c to the correct patch, which prevents segfaults on double free and fails an assert instead.

besser82 on 14 Dec 2017

👍2

json-c just got updated to 0.13 on ArchLinux. Now what?