Потокобезопасный, реентерабельный, безопасный для асинхронных сигналов putenv

Я заранее извиняюсь за то, что это будет немного дампа кода, я обрезал как можно больше неважного кода:

// Global vars / mutex stuff
extern char **environ;
pthread_mutex_t env_mutex = PTHREAD_MUTEX_INITIALIZER;

int
putenv_r(char *string)
{
    int len;
    int key_len = 0;
    int i;

    sigset_t block;
    sigset_t old;

    sigfillset(&block);
    pthread_sigmask(SIG_BLOCK, &block, &old);

    // This function is thread-safe
    len = strlen(string);

    for (int i=0; i < len; i++) {
        if (string[i] == '=') {
            key_len = i; // Thanks Klas for pointing this out.
            break;
        }
    }
    // Need a string like key=value
    if (key_len == 0) {
        errno = EINVAL; // putenv doesn't normally return this err code
        return -1;
    }

    // We're moving into environ territory so start locking stuff up.
    pthread_mutex_lock(&env_mutex);

    for (i = 0; environ[i] != NULL; i++) {
        if (strncmp(string, environ[i], key_len) == 0) {
            // Pointer assignment, so if string changes so does the env. 
            // This behaviour is POSIX conformant, instead of making a copy.
            environ[i] = string;
            pthread_mutex_unlock(&env_mutex);
            return(0);
        }
    }

    // If we get here, the env var didn't already exist, so we add it.
    // Note that malloc isn't async-signal safe. This is why we block signals.
    environ[i] = malloc(sizeof(char *));
    environ[i] = string;
    environ[i+1] = NULL;
    // This ^ is possibly incorrect, do I need to grow environ some how?

    pthread_mutex_unlock(&env_mutex);
    pthread_sigmask(SIG_SETMASK, &old, NULL);

    return(0);
}

Как следует из названия, я пытаюсь закодировать потокобезопасную, безопасную для асинхронных сигналов реентерабельную версию putenv. Код работает в том смысле, что он устанавливает переменную среды, как putenv, но у меня есть несколько проблем:

  1. Мой метод сделать его безопасным для асинхронных сигналов кажется немного неуклюжим, просто блокируя все сигналы (конечно, кроме SIGKILL/SIGSTOP). Или это самый правильный способ сделать это.
  2. Не является ли место блокировки моего сигнала слишком консервативным? Я знаю, что strlen не гарантирует безопасность асинхронного сигнала, а это означает, что моя блокировка сигнала должна произойти заранее, но, возможно, я ошибаюсь.
  3. Я почти уверен, что это потокобезопасно, учитывая, что все функции потокобезопасны и что я блокирую взаимодействие с environ, но я хотел бы, чтобы было доказано обратное.
  4. Я действительно не слишком уверен в том, является ли он реентерабельным или нет. Хотя это не гарантируется, я полагаю, что если я отмечу два других поля, он, скорее всего, будет реентерабельным?

Я нашел другое решение этого вопроса здесь, в котором они просто устанавливают соответствующую блокировку сигнала и блокировку мьютекса (больные рифмы), а затем вызывают putenv в обычном режиме. Это действительно? Если это так, то это, очевидно, намного проще, чем мой подход.

Извините за большой блок кода, надеюсь, я установил MCVE. Для краткости мне не хватает проверки ошибок в моем коде. Спасибо!


Вот остальная часть кода, включая основной, если вы хотите проверить код самостоятельно:

#include <string.h>
#include <errno.h>
#include <pthread.h>
#include <stdlib.h>
#include <stdio.h>
#include <signal.h>

// Prototypes
static void thread_init(void);
int putenv_r(char *string);

int
main(int argc, char *argv[]) {

    int ret = putenv_r("mykey=myval");
    printf("%d: mykey = %s\n", ret, getenv("mykey"));

    return 0;
}

person Daniel Porteous    schedule 21.12.2016    source источник
comment
Не по теме, но... так как C имеет массивы с нулевым отсчетом, это должно быть key_len = i;, а не key_len = i - 1;   -  person Klas Lindbäck    schedule 21.12.2016
comment
@KlasLindbäck Я использую i - 1, потому что хочу strncmp key часть key=value. i в момент разрыва цикла находится в позиции знака =, поэтому мне нужно вычесть 1 из индекса.   -  person Daniel Porteous    schedule 21.12.2016
comment
strncmp требуется количество символов, а не индекс последнего символа. Если у вас есть foo=bar, вы получите i = 3 и key_len = 2, а ваш strncmp будет сравнивать только два первых символа...   -  person Klas Lindbäck    schedule 21.12.2016
comment
@KlasLindbäck Ах, конечно, спасибо, что указали на это. Как вы сказали, не по теме, но хорошая ошибка, чтобы исправить!   -  person Daniel Porteous    schedule 21.12.2016
comment
Какое отношение malloc() к блокировке сигналов имеет небезопасность асинхронного сигнала? Я что-то упускаю.   -  person Andrew Henle    schedule 21.12.2016
comment
Возможно, я путаю это с повторным входом. Разве не правда, что если malloc прерывается (возможно, по сигналу, отсюда и мое замешательство), то его нельзя безопасно возобновить, потому что это может испортить стек?   -  person Daniel Porteous    schedule 21.12.2016
comment
@DanielPorteous Если malloc() будет прерван сигналом, он будет невидим для вызывающего абонента.   -  person Andrew Henle    schedule 21.12.2016
comment
Правильно. Я думаю, что меня смутило то, что мне не разрешено вызывать malloc() из обработчика сигнала, и в этом случае небезопасность асинхронного сигнала является проблемой.   -  person Daniel Porteous    schedule 21.12.2016


Ответы (1)


Этот код является проблемой:

// If we get here, the env var didn't already exist, so we add it.
// Note that malloc isn't async-signal safe. This is why we block signals.
environ[i] = malloc(sizeof(char *));
environ[i] = string;

Он создает char * в куче, присваивает адрес этого char * environ[i], затем перезаписывает это значение адресом, содержащимся в string. Это не сработает. Это не гарантирует, что environ впоследствии завершается NULL.

Потому что char **environ — это указатель на массив указателей. Последний указатель в массиве — NULL — так код может сказать, что он достиг конца списка переменных среды.

Что-то вроде этого должно работать лучше:

unsigned int envCount;

for ( envCount = 0; environ[ envCount ]; envCount++ )
{
    /* empty loop */;
}

/* since environ[ envCount ] is NULL, the environ array
   of pointers has envCount + 1 elements in it */
envCount++;

/* grow the environ array by one pointer */
char ** newEnviron = realloc( environ, ( envCount + 1 ) * sizeof( char * ) );

/* add the new envval */
newEnviron[ envCount - 1 ] = newEnvval;

/* NULL-terminate the array of pointers */
newEnviron[ envCount ] = NULL;

environ = newEnviron;

Обратите внимание, что проверки ошибок нет, и предполагается, что исходный массив environ был получен с помощью вызова malloc() или аналогичного. Если это предположение неверно, поведение не определено.

person Andrew Henle    schedule 21.12.2016
comment
Я просто использую environ, так как он поставляется естественным образом после вызова execve, я не уверен, что это влечет за собой malloc. Спасибо, что указали на это, у меня было впечатление, что способ, которым я добавлял новый элемент в environ, был недостаточным. - person Daniel Porteous; 21.12.2016
comment
Я обновлю свой вопрос, чтобы отразить эти предложения, возможно, обойдя шаги newEnviron и просто работая напрямую с environ. Я чувствую, что у кода все еще есть пути. - person Daniel Porteous; 21.12.2016
comment
@DanielPorteous Если вы работаете непосредственно с самим environ, код не будет потокобезопасным. Строго говоря, даже environ = newEnviron; не обязательно является потокобезопасным, поскольку обновление значения указателя не обязательно является атомарной операцией. - person Andrew Henle; 21.12.2016
comment
Даже если я использую мьютекс для управления доступом к окружающей среде? Пока я удостоверяюсь, что все мои потоки также соблюдают эти мьютексы, это будет потокобезопасным, да? Если нет, если это не слишком большая проблема, не могли бы вы объяснить, почему это не было бы? - person Daniel Porteous; 21.12.2016
comment
@DanielPorteous Да, даже если вы используете мьютекс. Потому что ваш мьютекс защищает только ваш код обновления, а не все, что читает среду, или любой код, который обновляет среду другими способами. Из-за того, что другие потоки могут читать среду, массив указателей, на который указывает environ, должен быть действительным в любое время. - person Andrew Henle; 21.12.2016
comment
Я вижу вашу точку зрения. Даже если бы мне удалось подтвердить, что все потоки не будут вести себя неправильно, гораздо надежнее минимизировать время небезопасного потока. Я обновлю код соответственно, спасибо. Конечно, я оставлю вопрос открытым еще на некоторое время, чтобы посмотреть, заметит ли кто-нибудь еще что-нибудь, но вскоре я отмечу ваш ответ как правильный, вы были невероятно полезны. - person Daniel Porteous; 21.12.2016
comment
Обновление: похоже, что окружение действительно не создается malloc по умолчанию, realloc вызывает большую ошибку недопустимого указателя. Возможно, придется вернуться к редактированию самого environ, несмотря на риски безопасности потока. Не знаю, как увеличить размер environ. - person Daniel Porteous; 21.12.2016
comment
@DanielPorteous Если вы используете glibc/Linux, исходный код для putenv() можно найти по адресу fossies.org/dox/glibc-2.24/putenv_8c_source.html и фактическую реализацию среды на fossies.org/dox/glibc-2.24/setenv_8c_source.html - person Andrew Henle; 21.12.2016