Toxcore and thread safe (question)

[hkarel]

In my application the toxcore worked in a separate thread. Whether it will be safe to call functions (for example tox_friend_by_public_key()) to work with the toxcore from another thread?

[nurupo]

Toxcore is not thread-safe, you must synchronize all access to it. You can safely access it in-between tox_iterate() calls or while inside callbacks, since callbacks pause the tox-iterate loop as they are being called from it. You should not access toxcore any other time, as you will be accessing it during the tox_iterate() call, which means that toxcore might change the data while you are reading it, which is not what you want to do.

The documentation has some explanation on thread-safety but it somehow misses to tell all this.

[hkarel]

Toxcore is not thread-safe, you must synchronize all access to it. You can safely access it in-between tox_iterate() calls or while inside callbacks, since callbacks pause the tox-iterate loop as they are being called from it. You should not access toxcore any other time, as you will be accessing it during the tox_iterate() call, which means that toxcore might change the data while you are reading it, which is not what you want to do.

Well, the logic is clear. But there is the following question: it turns out that toxav instance has to live in the same thread, as tox, because the toxav functions is directly address to tox data fields without any blocking. For example: toxav_answer() -> m_friend_exists().
Or, all toxav functions should be called from the tox thread?

[nurupo]

Sorry, not familiar with toxav, maybe someone else can answer this.

[dvor]

See documentation in toxav.h

Unlike the Core API, this API is fully thread-safe. The library will ensure the proper synchronization of parallel calls.

A common way to run ToxAV (multiple or single instance) is to have a thread, separate from tox instance thread, running a simple toxav_iterate loop, sleeping for toxav_iteration_interval * milliseconds on each iteration.

[hkarel]

Unlike the Core API, this API is fully thread-safe. The library will ensure the proper synchronization of parallel calls.

Yes, I read this. But let's look inside the function, for example:

bool toxav_audio_send_frame(ToxAV *av, uint32_t friend_number, ...) 
{
    TOXAV_ERR_SEND_FRAME rc = TOXAV_ERR_SEND_FRAME_OK;
    ToxAVCall *call;
    if (m_friend_exists(av->m, friend_number) == 0) {
        rc = TOXAV_ERR_SEND_FRAME_FRIEND_NOT_FOUND;
        goto END;
    }

    if (pthread_mutex_trylock(av->mutex) != 0) {
        rc = TOXAV_ERR_SEND_FRAME_SYNC;
        goto END;
    }

    call = call_get(av, friend_number);

    if (call == nullptr || !call->active || call->msi_call->state != msi_CallActive) {
        pthread_mutex_unlock(av->mutex);
        rc = TOXAV_ERR_SEND_FRAME_FRIEND_NOT_IN_CALL;
        goto END;
    }

    if (call->audio_bit_rate == 0 ||
            !(call->msi_call->self_capabilities & msi_CapSAudio) ||
            !(call->msi_call->peer_capabilities & msi_CapRAudio)) {
        pthread_mutex_unlock(av->mutex);
        rc = TOXAV_ERR_SEND_FRAME_PAYLOAD_TYPE_DISABLED;
        goto END;
    }

    pthread_mutex_lock(call->mutex_audio);
    pthread_mutex_unlock(av->mutex);
...

@dvor, you think that calling m_friend_exists(av-> m, friend_number) is thread-safe?
Let's take a deep look: m_friend_exists() -> friend_not_valid(). A direct appeal to m->friendlist without any locks is really safe?
It will only be safe in two cases:

1. m->friendlist does not change in the during executed the program (what isn't the truth in our case)
2. The function toxav_audio_send_frame() called only from tox-thread (In this case, there are problems with synchronization intervals, and the transmitted sound becomes very bad).

Here I would also like to understand: maybe I don't see some moments, and my fears are vain.

[zoff99]

@hkarel some more info to threading and AV:

https://github.com/zoff99/c-toxcore/blob/zoff99/_0.1.10_2017_video_fix_10p/docs/toxav_threading.md

[hkarel]

@zoff99, judging by the code, the toxav data is protected from race conditions, but for tox the probability of race conditions is present, see the code example above. By the way, in your example the function toxav_audio_send_frame() call is shown.
Imagine that a new friend is added in the thread [Main Thread]. And at this operation there is a memory reallocation for m->friendlist. At the same time, from the thread [Some Client Thread] called a function toxav_audio_send_frame() with direct access to m->friendlist

[nurupo]

I agree with @hkarel's concerns here, this m_friend_exists() call in toxav_audio_send_frame() does look like a race condition to me.

[zoff99]

for these cases making realloc_friendlist() should be sufficent

[hkarel]

In my project, I solved this problem as follows:
for each tox-function is used the construction

        uint32_t friendNum;
        { //Block for ToxGlobalLock
            ToxGlobalLock toxGlobalLock; (void) toxGlobalLock;
            friendNum = tox_friend_add_norequest(_tox, (uint8_t*)pubKey.constData(), 0);
        }

At the tox-core thread loop

        ...
        { //Block for ToxGlobalLock
            ToxGlobalLock toxGlobalLock; (void) toxGlobalLock;
            tox_iterate(_tox, this);
        }
        ...

Inside tox-callback functions the ToxGlobalLock locker not used.

[iphydf]

Toxcore is not aiming to be thread-safe. You can use a lock outside toxcore to lock the whole Tox instance.