Occasional segfaults in __lambda65_ when liking tweets

Describe the bug
Cawbird occasionally crashes when trying to like a tweet. I’ve only noticed this happening today (twice), but the package manager says I installed the current version on Monday (2020-09-28), so maybe something changed on Twitter’s end.

To Reproduce

Right-click a tweet.
Click the “like” button.

Actual behavior
Very occasionally (I like plenty of tweets and only had two crashes so far), the tweet will stay in the right-clicked view for some seconds (presumably while waiting for the response from Twitter), and then cawbird will crash.

System details:

OS: Arch Linux
Version: 1.2.1.r2.g3a5e263a-1 (commit 3a5e263)
Package type: AUR -git package

Additional context
coredumpctl gdb for the latest crash reports that it happened at src/util/TweetUtils.vala:261 – the “Faving and we didn’t get ‘already Faved’” line:

      try {
        call.invoke_async.end (res);
      } catch (GLib.Error e) {
        var tmp_error = failed_request_to_error (call, e);
        // If we can handle it cleanly, pretend it worked                                                                                         
        if ((status && tmp_error.code != 139) // Faving and we didn't get "already Faved"                                                         
           ||
            (!status && tmp_error.code != 144)) { // or un-Faving and didn't get "no such status"                                                 
          err = tmp_error;
          set_favorite_status.callback ();
          return;
        }
      }

p tmp_error reveals that it is 0x0, hence the segfault. e is not null, on the other hand, so it looks like failed_request_to_error turned a non-null error into a null one – it’s not obvious to me where it would do that, though.

Right around the segfault, there are some warnings in the journal (I assume the ordering isn’t fully correct):

cawbird[30044]: json_node_get_object: assertion 'JSON_NODE_IS_VALID (node)' failed
audit[30044]: ANOM_ABEND auid=1000 uid=1000 gid=1000 ses=2 pid=30044 comm="cawbird" exe="/usr/bin/cawbird" sig=11 res=1
cawbird[30044]: json_object_get_array_member: assertion 'object != NULL' failed
cawbird[30044]: json_array_get_element: assertion 'array != NULL' failed
cawbird[30044]: json_node_get_object: assertion 'JSON_NODE_IS_VALID (node)' failed
kernel: cawbird[30044]: segfault at 4 ip 000055da0aec151d sp 00007fffe7bb7d80 error 4 in cawbird[55da0ae6f000+106000]
kernel: Code: cd d5 ff ff 48 89 45 c0 48 8b 45 c0 48 89 45 a8 48 8b 85 78 ff ff ff 8b 40 28 85 c0 74 1f 48 8b 45 a8 48 89 45 c8 48 8b 45 c8 <8b> 40 04 3d 8b 00 00 00 0f 95 c0 0f b6 c0 89 45 88 eb 07 c7 45 88
kernel: audit: type=1701 audit(1601663359.319:451): auid=1000 uid=1000 gid=1000 ses=2 pid=30044 comm="cawbird" exe="/usr/bin/cawbird" sig=11 res=1
cawbird[30044]: json_object_get_int_member: assertion 'object != NULL' failed
cawbird[30044]: json_object_get_string_member: assertion 'object != NULL' failed
cawbird[30044]: g_error_new_literal: assertion 'message != NULL' failed

The JSON messages might indicate that the Twitter error response is supposed to be valid JSON, but for whatever reason it’s not, which confuses Cawbird?

Okay, I should’ve looked at those assertion messages more closely, they correspond directly to the end of the code of failed_request_to_error:

cawbird[30044]: json_node_get_object: assertion 'JSON_NODE_IS_VALID (node)' failed
cawbird[30044]: json_object_get_array_member: assertion 'object != NULL' failed
cawbird[30044]: json_array_get_element: assertion 'array != NULL' failed
cawbird[30044]: json_node_get_object: assertion 'JSON_NODE_IS_VALID (node)' failed
cawbird[30044]: json_object_get_int_member: assertion 'object != NULL' failed
cawbird[30044]: json_object_get_string_member: assertion 'object != NULL' failed
cawbird[30044]: g_error_new_literal: assertion 'message != NULL' failed

      var parser = new Json.Parser();
      parser.load_from_data (json);
      var obj = parser.get_root().get_object();
      var errors = obj.get_array_member("errors");
      // Assume there's always at least one, and we just take the first                                                                           
      var error = errors.get_element(0).get_object();
      // Twitter's error codes don't go above three digits - https://developer.twitter.com/en/docs/basics/response-codes                          
      var code = (int)error.get_int_member("code");
      var message = error.get_string_member("message");
      return new GLib.Error.literal (get_error_domain(), code, message);

parser.get_root() seems to be null, and from there on you can follow all the individual calls in the warning messages (the relationship between the Vala and C function names seems pretty clear). And just to make sure, I also confirmed manually that g_error_new_literal (which new GLib.Error.literal(…) presumably compiles to) returns null when called with a null message:

#include <glib.h>
#include <stdio.h>

void main() {
  printf("%p\n", g_error_new_literal(g_quark_from_static_string(""), 123, (char*) NULL));
}

$ cc -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -lglib-2.0 tmp.c -o tmp
$ ./tmp

(process:34930): GLib-CRITICAL **: 20:59:42.998: g_error_new_literal: assertion 'message != NULL' failed
(nil)

I think I have a fix, currently running a patched version of cawbird to see if the crash still happens :)

Thanks for investigating. A bit odd that we might get a non-JSON response and not get an error code, but if it does happen then I can imagine it would crash out in a horrible mess!

So far I haven’t had another crash but I also haven’t seen the new error message I added in the output. I suspect Twitter was just temporarily sending weird error responses (which would also be why I only started seeing the error today), and they’ve fixed it on their end.

Here’s the diff – I’ll probably send it as a proper pull request tomorrow or so:

diff --git a/src/util/TweetUtils.vala b/src/util/TweetUtils.vala
index 900f68ed..eedf095f 100644
--- a/src/util/TweetUtils.vala
+++ b/src/util/TweetUtils.vala
@@ -57,7 +57,11 @@ namespace TweetUtils {
        */
       var parser = new Json.Parser();
       parser.load_from_data (json);
-      var obj = parser.get_root().get_object();
+      var node = parser.get_root();
+      if (node == null) {
+          return new GLib.Error (get_error_domain(), 0, "Twitter error is not valid JSON: %s", json);
+      }
+      var obj = node.get_object();
       var errors = obj.get_array_member("errors");
       // Assume there's always at least one, and we just take the first
       var error = errors.get_element(0).get_object();

“0” is probably not a good error code, but I wasn’t sure what else it should be.

I think that's reasonable - 0 can be our "unknown error" code. It saves us having to try and avoid conflicting with other Twitter error codes. The code_to_message will then fall through to the default and return the string that you supplied. The only down-side is that it won't be translated.

131, “Internal error”, might also fit, but maybe it’s better to not reuse a Twitter code, yeah.

Don’t crash if Twitter error is not valid JSON

Navigation Menu

Repository navigation

Occasional segfaults in __lambda65_ when liking tweets #200

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Occasional segfaults in __lambda65_ when liking tweets #200

Description

Activity

lucaswerkmeister commented on Oct 2, 2020

lucaswerkmeister commented on Oct 2, 2020

IBBoard commented on Oct 2, 2020

lucaswerkmeister commented on Oct 2, 2020

IBBoard commented on Oct 3, 2020

lucaswerkmeister commented on Oct 3, 2020

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions