Go to Qiita Advent Calendar 2024 Top
LoginSignup
11
6

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 5 years have passed since last update.

@knknkn1162(Kenta Nakajima)

C言語のpthread_createで嵌ったお話

Last updated at Posted at 2018-08-13

はじめに

ちょうど、TLPI(The Linux Programming Interface)やっていて、以下のコード(Listing 30-4)に出くわした。

引数の数字の秒数分sleepして、返事する処理だが、multithreadになっている：

$ ./a.out 1 1 2 3 3
start
Reaped thread 0(numLive=4) # <= 1秒後
Reaped thread 1(numLive=3)
Reaped thread 2(numLive=2) # <= 2秒後
Reaped thread 3(numLive=1) # <= 3秒後
Reaped thread 4(numLive=0)
$

となってほしい。

以下、本に載っていたコード：(ちょっと細部違うが本質的には同値です)

Listing30_4
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

static pthread_cond_t threadDied = PTHREAD_COND_INITIALIZER;
static pthread_mutex_t threadMutex = PTHREAD_MUTEX_INITIALIZER;


static int totThreads = 0;
static int numLive = 0;

static int numUnjoined = 0;

enum tstate {
  TS_ALIVE,
  TS_TERMINATED,
  TS_JOINED
};

struct threadInfo {
  pthread_t tid;
  enum tstate state;
  int sleepTime;
};

static struct threadInfo* threads;

static void* threadFunc(void* arg);

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

  int s, idx;
  threads = calloc(argc-1, sizeof(struct threadInfo));
  if(threads == NULL) {
    perror("calloc");
    exit(1);
  }

  for(idx = 0; idx < argc - 1; idx++) {
    threads[idx].sleepTime = atoi(argv[idx+1]);
    threads[idx].state = TS_ALIVE;
    // int pthread_create(pthread_t * thread, pthread_attr_t * attr, void * (*start_routine)(void *), void * arg);
    s = pthread_create(&threads[idx].tid, NULL, threadFunc, (void*)&idx);
    if(s != 0) {
      perror("pthread_create");
      exit(1);
    }
  }

  totThreads = argc - 1;
  numLive = totThreads;

  printf("start\n");
  while(numLive > 0) {
    s = pthread_mutex_lock(&threadMutex);
    if(s != 0) {
      perror("pthread_mutex_lock");
    }

    while(numUnjoined == 0) {
      // These functions atomically release mutex and cause the calling thread to block on the condition variable cond
      s = pthread_cond_wait(&threadDied, &threadMutex);
      if(s != 0) {
        perror("pthread_cond_wait");
        exit(1);
      }
    }

    for(idx = 0; idx < totThreads; idx++) {
      if(threads[idx].state == TS_TERMINATED) {
        s = pthread_join(threads[idx].tid, NULL);
        if(s != 0) {
          perror("pthread_join");
        }
        threads[idx].state = TS_JOINED;
        numLive--;
        numUnjoined--;

        printf("Reaped thread %d(numLive=%d)\n", idx, numLive);
      }
    }

    s = pthread_mutex_unlock(&threadMutex);
    if(s != 0) {
      perror("pthread_mutex_unlock");
      exit(1);
    }
  }
  exit(EXIT_SUCCESS);
}

static void* threadFunc(void* arg) {
  int s;
  int idx = *((int*)arg);
  
  sleep(threads[idx].sleepTime);
  s = pthread_mutex_lock(&threadMutex);
  numUnjoined++;
  threads[idx].state = TS_TERMINATED;
  s = pthread_mutex_unlock(&threadMutex);

  s = pthread_cond_signal(&threadDied);
  return NULL;
}

結論から言うと、このコードにはバグがある。

バグの詳細

pthread_create(&threads[idx].tid, NULL, threadFunc, (void*)&idx);
の部分でidxの変数を使いまわしているので、threadFuncの内部のint idx = *((int*)arg);の評価で正しくidxを拾ってきていない。
つまり、

0. [mainThread]idx = 0の状態
1. [mainThread, pthread_create] (void*)&idxでアドレスAを引数に
2. [mainThread] idx = 1にincrementされる。(&idxは変化しないことに注意すること)
3. [threadFunc] `int idx = *((int*)arg);`でidx <- 1となってしまう(初めてthreadFuncを呼び出すので、idx=0となっているはず)
4. [threadFunc] threads[idx].sleepTimeが`threads[1].sleepTime`で評価される。(callocしているので、0と評価されてしまう)

となる。

または、

0. [mainThread]idx = 0の状態
1. \[mainThread, pthread_create\] (void*)&idxでアドレスAを引数に
2. [threadFunc] `(int*)arg`がアドレスAと評価される
3. [mainThread] idx = 1にincrementされる
4. [threadFunc] *((int*)arg);`でidx <- 1となってしまう
5. threads[idx].sleepTimeが`threads[1].sleepTime`で評価される。(callocしているので、0と評価されてしまう。)

でダメ。

ダメな修正例

まずは行き当たりばったりの回答から。

wrong.c
  // skip
  for(idx = 0; idx < argc - 1; idx++) {
    int tmp = idx;
    threads[idx].sleepTime = atoi(argv[idx+1]);
    threads[idx].state = TS_ALIVE;
    // int pthread_create(pthread_t * thread, pthread_attr_t * attr, void * (*start_routine)(void *), void * arg);
    s = pthread_create(&threads[idx].tid, NULL, threadFunc, (void*)&tmp);
    if(s != 0) {
      perror("pthread_create");
      exit(1);
    }
  }
  // skip

int tmp = idx;と一時変数にしてしのいでいるように見えるが、2つの点でダメ。

  1. int tmp = idx;はループ終わったら解放される。アドレスが解放された後、threadFuncで参照するのダメ。
  2. 1回目のループの&tmpと2回目のループの&tmpは異なるとは限らない(というか、自分の環境(gcc version 4.8.4 (Ubuntu 4.8.4-2ubuntu1~14.04.4))では同じになった)。と言うことで、こんな一時しのぎのコード書いても最初の例のコードと全く同じ挙動になってしまう。

どうすればよい？

一つ目のとても単純な方法は、あらかじめstack領域にpthread_createの第4引数用の領域を確保しておく。発想としては、ループによって、参照の指し示す先が予期せず変わるのならば、あらかじめthreadごとにメモリ確保しておけばよいと言うものだ。

.c
// skip
#define THREAD_MAX  256
int main(int argc, char *argv[])
  int index[THREAD_MAX];
  
  threads = calloc(argc-1, sizeof(struct threadInfo));
  if(threads == NULL) {
    perror("calloc");
    exit(1);
  }

  for(idx = 0; idx < argc - 1; idx++) {
    index[idx] = idx;
    threads[idx].sleepTime = atoi(argv[idx+1]);
    threads[idx].state = TS_ALIVE;
    // int pthread_create(pthread_t * thread, pthread_attr_t * attr, void * (*start_routine)(void *), void * arg);
    s = pthread_create(&threads[idx].tid, NULL, threadFunc, (void*)&index[idx]);
    if(s != 0) {
      perror("pthread_create");
      exit(1);
    }
  }
  // skip

これでうまくいく。mainで　int index[THREAD_MAX];を作ったので、mainが終わるまでメモリ確保され、threadFuncのidx変数は予期せぬ値にならない。

ただ、スレッドがどれくらい立ち上がるのか不明な状況で、THREAD_MAXをかなり大きくして、int index[THREAD_MAX];とそれなりに大きいメモリを確保するの、勿体無い。

ちなみに自分は、pthread_createのループの部分でpthread_cond_wait, pthread_cond_signalの同期処理を取れば良いと思い、以下のようにした。

myanswer.c
// skip
static pthread_cond_t threadAllocated = PTHREAD_COND_INITIALIZER;

int main(int argc, char *argv[]) {
  //skip
  for(idx = 0; idx < argc - 1; idx++) {
    threads[idx].sleepTime = atoi(argv[idx+1]);
    threads[idx].state = TS_ALIVE;
    pthread_mutex_lock(&threadMutex);
    // int pthread_create(pthread_t * thread, pthread_attr_t * attr, void * (*start_routine)(void *), void * arg);
    s = pthread_create(&threads[idx].tid, NULL, threadFunc, (void*)&idx);
    if(s != 0) {
      perror("pthread_create");
      exit(1);
    }
    pthread_cond_wait(&threadAllocated, &threadMutex);
    pthread_mutex_unlock(&threadMutex);
  }
  // skip
  exit(EXIT_SUCCESS);

}

static void* threadFunc(void* arg) {
  int s;
  int idx;
  
  s = pthread_mutex_lock(&threadMutex);
  idx = *((int*)arg);
  s = pthread_mutex_unlock(&threadMutex);
  s = pthread_cond_signal(&threadAllocated);

  sleep(threads[idx].sleepTime);
  s = pthread_mutex_lock(&threadMutex);
  numUnjoined++;
  threads[idx].state = TS_TERMINATED;
  s = pthread_mutex_unlock(&threadMutex);

  s = pthread_cond_signal(&threadDied);
  return NULL;
}

これで正常に動く。でもなんか微妙だな、と思った。。
と言うのも、pthread_createの周辺にlockをおくのがまずイケていないし、threadFuncの内部で2度lock, unlockしているのもなんかなぁ、と言う感じ。

なんかいい方法ないかな、と思って http://man7.org/tlpi/errata/index.html をみたら、なるほど~ と言う訂正内容が載っていた：

TLPIの修正内容

http://man7.org/tlpi/errata/index.html の修正ポイントは2つ。

  1. pthread_createの第4引数を(void*)idxとする。
  2. uintptr_tを用いる(stdint.hをインクルードする)

1番目の修正でidxがループで切り替わっても問題ない。idxを値渡ししているため、コピーされているから。(最初の例は、参照を渡してしまっているからmainThreadのidxのincrementに追従してしまうことが原因でバグを引き起こしているのであった)

1番目の修正だけだと、warningが出る。pthread_createでvoid*型からint型へのキャストがだいじょうぶ？というwarningだ。
整数型からポインタ型へのキャストを問題なく行うためには、intptr_tおよび、unintptr_tを用いる( https://www.jpcert.or.jp/sc-rules/c-int36-c.html )も参照：

tlpi.c
// skip
int main(int argc, char *argv[]) {
  uintptr_t idx;
  // skip
  // ~
  for(idx = 0; idx < argc - 1; idx++) {
    threads[idx].sleepTime = atoi(argv[idx+1]);
    threads[idx].state = TS_ALIVE;
    //pthread_mutex_lock(&threadMutex);
    // int pthread_create(pthread_t * thread, pthread_attr_t * attr, void * (*start_routine)(void *), void * arg);
    s = pthread_create(&threads[idx].tid, NULL, threadFunc, (void*)idx);
    if(s != 0) {
      perror("pthread_create");
      exit(1);
    }
    /* pthread_cond_wait(&threadAllocated, &threadMutex); */
    /* pthread_mutex_unlock(&threadMutex); */
  }
  // ~
  // skip
  // ~~
  exit(EXIT_SUCCESS);
}

static void* threadFunc(void* arg) {
  int s;
  uintptr_t idx;
  
  idx = (uintptr_t)arg;

  sleep(threads[idx].sleepTime);
  s = pthread_mutex_lock(&threadMutex);
  numUnjoined++;
  threads[idx].state = TS_TERMINATED;
  s = pthread_mutex_unlock(&threadMutex);

  s = pthread_cond_signal(&threadDied);
  return NULL;
}

このコードも正常に動く。

著者も嵌るスレッドという物のあんなるを.. と言うお話でした。

注意：

今回は、pthread_createの第4引数にint型を渡しているところから端を発しているため、ある種局所的なお話だと思う。
通常、threadを使用するときは、thread poolとして実装することが多いので、そのときは、
https://github.com/knknkn1162/c_sample/blob/36f51a747c6aece6c82a46f4d78f0a167f140b2f/worker_thread2.c#L95
のように、threadpool型のポインタをpthread_createに突っ込むのが良いと思われる。1

個人的には、著者も間違えているなぁ、と言うことで興味をそそられたのと、それに対する解決方法が綺麗だ、ということでメモっておこうという意図があったのでした。

  1. https://www.amazon.co.jp/Pthreads%E3%83%97%E3%83%AD%E3%82%B0%E3%83%A9%E3%83%9F%E3%83%B3%E3%82%B0-Bradford-Nichols/dp/4900900664 を参照にして自作した ↩

11
6
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
11
6

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?