Segfault due to race conditions in discord_cleanup when called from another thread about concord

[InterLinked1]

Okay, so I'm getting some presence info now, but it seems to fail halfway through, for example starting around member 34 of 77 in one test, event->presences->array + i starts pointing to invalid memory, and I get a segfault.

#4  0x00007f84903fd306 in on_guild_members_chunk (client=0x555a8c44c5c0, event=0x7f84800d25f0) at mod_discord.c:314
        member = 0x7f84800cbb20
        user = 0x7f84800c6210
        presence = 0x7f84800c9540
        presencestatus = 0x656c6469 <error: Cannot access memory at address 0x656c6469>
        i = 36
        members = 0x7f84800c5f40
        presences = 0x7f84800c8d40
        __PRETTY_FUNCTION__ = "on_guild_members_chunk"
        __FUNCTION__ = "on_guild_members_chunk"

Printing out the sizes, I get an interesting result: 77 members, 34 presences

Is it expected that presences->size might be different from members->size? That seems odd to me. Obviously, I'm reading invalid memory, but I wonder why the number of presences would be less than the total number of members, and the rest is missing.

[lcsmuller]

Your suggested solution makes sense, ccord_has_sigint is checked from the main thread and when true it will trigger a discord_shutdown(). This should ensure proper shutdown in a thread-safe manner. I hope to look into a good solution for this in the following days.

I initially had this ordering:

Yeah, this ordering will lead to issues as we need to ensure discord_run() has been returned from, before doing a discord_cleanup()

The workflow is:

1. discord_shutdown() will notify concord that it should cease the connection with Discord, this is done asynchronously, meaning that even if discord_shutdown() returns we'll still be in the process of shutting down
2. concord waits until we have confirmation from Discord that the connection has been closed
3. discord_run() returns
4. we may now call discord_cleanup()

So discord_shutdown() is asynchronous, all it does is notify concord that it SHOULD shut down sometime in the near future... We'll only know for sure the bot has shutdown once discord_run() returns

[InterLinked1]

Thanks for confirming!

A couple other clarifications from trying to figure these out the past couple days, and I can certainly open a new issue if this is a legitimate issue, not just user error, but I wonder if these are other issues that may exist:

2 things I'm struggling to do:

• obtain user presences on startup
• determine what users are in each channel in a guild

Luckily, both of these only need to be done once.

I can get presence updates just fine, but not actually the initial presences when the module starts, so I only accumulate presences over time, but don't have the initial presences. It seems this should be possible, but all the APIs either return NULL or don't have this implemented. Am I doing this wrong?

discord_request_guild_members doesn't seem to work as the returned members' presences are all NULL, even though the event includes it:

struct discord_presence_updates *presences = event->presences;
for (i = 0; i < members->size; i++) {
		struct discord_guild_member *member = &members->array[i];
		struct discord_user *user = member->user;
		struct discord_presence_update *presence = presences ? &presences->array[i] : NULL; /* Initial (current) presence status */
	}
}

I did notice some other events seem to return NULL for certain fields that other functions do include, so I thought maybe this was for efficiency reasons (don't want to fetch more than we need to).

This suggests guild member would be the right API to use, since it has presence: https://discord.js.org/#/docs/main/stable/class/GuildMember

But, according to the documentation for this, there is no presence field, even though it would seem this would be the logical place for it: https://cogmasters.github.io/concord/structdiscord__guild__member.html

There is also this one:

https://discord.com/developers/docs/resources/guild#guild-widget-object
https://cogmasters.github.io/concord/structdiscord__guild__widget.html

But, looking at what the response contains, I can't find any fields that actually contain any presence data. Certainly, struct discord_user * does not.

I've scoured both the code and the documentation, but have come up short on other ways to get a guild member's presence on demand, which seems necessary if there's no way to get it from discord_request_guild_members (which would be most ideal, if possible, to avoid making a separate request for every user).

2. I'm running into a similar issue with discord_get_channel:

struct discord_ret_channel ret2 = { .sync = &dchannel };
code = discord_get_channel(client, channel->id, &ret2);
assert(dchannel.member);

The assertion fails, because the member field is always NULL.
I admit I don't understand why certain APIs are always returning fields that are always NULL, but maybe I'm not understanding some usage aspect of this.

Yes, the relevant intents should be set:
discord_add_intents(discord_client, DISCORD_GATEWAY_MESSAGE_CONTENT | DISCORD_GATEWAY_GUILD_MESSAGES | DISCORD_GATEWAY_GUILD_PRESENCES | DISCORD_GATEWAY_GUILDS | DISCORD_GATEWAY_GUILD_MEMBERS | DISCORD_GATEWAY_DIRECT_MESSAGES | DISCORD_GATEWAY_PRESENCE_UPDATE);

And all of those are toggled on the bot side as well, so I don't think it's permissions.

Any thoughts on this? Are these even the right APIs to use?

[lcsmuller]

@InterLinked1 I can take a look at those separate issues later on, in the meantime can you confirm that the JSON payloads sent by Discord do in fact contain the field you're looking for? Please ensure that:

1. Concord's logging is enabled, for that you'll need to start your bot with discord_config_init()
2. Locate the JSON payload sent by Discord at your generated http.log file.

This suggests guild member would be the right API to use, since it has presence: https://discord.js.org/#/docs/main/stable/class/GuildMember

On another note, I see that you are using djs as a reference as to what to expect from the Concord's API, instead may I suggest you take a look at the official Discord API? Our goal is to keep concord a 1:1 mapping to the official API, so if you spot something there and seems to be missing from Concord chances are we need to have it be updated.

According to the Discord API, there is no presence field for guild members, which means djs must fetch that internally in some other way.

[InterLinked1]

From what I can see, yes, it does appear the presences are getting sent, but either I'm not using the library right or the library isn't exposing these:

This is the beginning of a line that contains a whole JSON array of presence data, which appears after a request to https://discord.com/api/v10/channels/<CHAN>

WS_RCV_TEXT [WEBSOCKETS] - 2023-02-5T19:36:23.687Z - wss://gateway.discord.gg/?v=10&encoding=json
{"t":"GUILD_CREATE","s":2,"op":0,"d":{"application_id":null,"mfa_level":0,"guild_hashes":{

The fact that a whole array of presences is coming through leads me to believe my first snippet is probably the right idea, but for some reason it's just NULL for me.

[lcsmuller]

There was indeed an issue with 'struct discord_request_guild_members', you can get the latest fix from the dev branch

I used the following snippet while working on a fix:

#include <stdio.h>
#include <stdlib.h>

#include <concord/discord.h>
#include <concord/log.h>

void
on_members_chunk(struct discord *client, const struct discord_guild_members_chunk *event)
{
    const int n = event->presences ? event->presences->size : 0;
    char text[8192];
    
    for (int i = 0; i < n; ++i) {
        discord_presence_update_to_json(text, sizeof(text), event->presences->array + i);
        log_info("%s\n", text);
    }
}

void
on_members_get(struct discord *client, const struct discord_message *event)
{
    if (event->author->bot) return;

    struct discord_request_guild_members request = {
        .guild_id = event->guild_id,
        .presences = true,
    };
    discord_request_guild_members(client, &request);
}

int main(void)
{
    ccord_global_init();
    struct discord *client = discord_config_init("config.json");

    discord_add_intents(client, DISCORD_GATEWAY_GUILD_PRESENCES
                                    | DISCORD_GATEWAY_GUILD_MEMBERS);
                                    
    discord_set_on_guild_members_chunk(client, &on_members_chunk);
    discord_set_on_command(client, "members_get", &on_members_get);

    discord_run(client);

    discord_cleanup(client);
    ccord_global_cleanup();
}

I'm running into a similar issue with discord_get_channel:

Now this solves the first issue you encountered, but it doesn't seem discord_get_channel() returns guild members from my tests... perhaps there is some other way the Discord API provides for fetching members via a channel?

[lcsmuller]

Do you mind opening this as a separate issue? Also, can you verify by the http.log payload that the presences and members array are indeed of the same size?