Reputation: 406
I'm trying to implement a shuffling technique based on a random time so that it is possible for one message to overtake the next message and becomes shuffled. I have an original message that is separated into chunks per thread. Each thread sleeps and prints out the chunk afterwards.
original_message > grab a slice of original_message > deal with it in another thread > grab another slice > deal with it in another NEW thread while the other thread may/may not be running > and repeat until all slice is taken.
However, sometimes the output is the same and that they do not output the after the sleep one by one e.g. the one with 0 seconds sleep comes out first.... How can I create a new thread with its own stack so that it does not share those variables. I'm not sure mutex lock will suffice because it may affect the sleep timings. What can I do to fix it?
#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <string.h>
#include <unistd.h>
struct argsparam {
int sleep;
char split_message[5];
};
void* sleep_shuffle(void *arguments){
struct argsparam *args = arguments;
sleep(args->sleep);
printf("shuffled message: %s\n", args->split_message);
}
int main(){
struct argsparam args;
pthread_t sleeppt[4];
time_t t;
srand((unsigned) time(&t));
char original_message[20] = "1234567890abcdefghij";
for(int i=0; i<4; i++){
memcpy(args.split_message , original_message+(i*5), 5);
args.split_message[5] = '\0'; //null terminator
args.sleep = rand() % 5;
printf("message: %s and time: %d\n", args.split_message, args.sleep);
if(pthread_create( &sleeppt[i], NULL, sleep_shuffle, (void *)&args) != 0)
printf("Error in creating thread!");
}
for(int i=0; i<4; i++){
pthread_join(sleeppt[i], NULL);
}
}
Upvotes: 1
Views: 1217
Reputation: 89
First, the size of the "split_message[5] of struct argsparam" array is incorrect.
Second, why not use "args" as an array?
--- orig.c 2017-06-14 03:18:25.569775282 +0900
+++ dest.c 2017-06-14 03:16:52.989777492 +0900
@@ -6,7 +6,7 @@
struct argsparam {
int sleep;
- char split_message[5];
+ char split_message[6];
};
void* sleep_shuffle(void *arguments){
@@ -16,7 +16,7 @@ void* sleep_shuffle(void *arguments){
}
int main(){
- struct argsparam args;
+ struct argsparam args[4];
pthread_t sleeppt[4];
time_t t;
int i;
@@ -25,12 +25,12 @@ int main(){
char original_message[20] = "1234567890abcdefghij";
for(i=0; i<4; i++){
- memcpy(args.split_message , original_message+(i*5), 5);
- args.split_message[5] = '\0'; //null terminator
- args.sleep = rand() % 5;
+ memcpy(args[i].split_message , original_message+(i*5), 5);
+ args[i].split_message[5] = '\0'; //null terminator
+ args[i].sleep = rand() % 5;
- printf("message: %s and time: %d\n", args.split_message, args.sleep);
- if(pthread_create( &sleeppt[i], NULL, sleep_shuffle, (void *)&args) != 0)
+ printf("message: %s and time: %d\n", args[i].split_message, args[i].sleep);
+ if(pthread_create( &sleeppt[i], NULL, sleep_shuffle, (void *)(args + i)) != 0)
printf("Error in creating thread!");
}
Upvotes: 1
Reputation: 223689
You're passing the same copy of args
to each thread, overwriting the previous value before calling the next thread, so what each thread reads is dependent on the timing of each thread.
Create an array of struct argsparam
, one for each thread, then pass in the pointer to the appropriate instance. That way they won't interfere with each other.
int main(){
// an array of args
struct argsparam args[4];
pthread_t sleeppt[4];
time_t t;
srand((unsigned) time(&t));
char original_message[20] = "1234567890abcdefghij";
for(int i=0; i<4; i++){
// assign to a specific array instance for each thread
memcpy(args[i].split_message , original_message+(i*5), 5);
args[i].split_message[5] = '\0'; //null terminator
args[i].sleep = rand() % 5;
printf("message: %s and time: %d\n", args[i].split_message, args[i].sleep);
if(pthread_create( &sleeppt[i], NULL, sleep_shuffle, (void *)&args[i]) != 0)
printf("Error in creating thread!");
}
for(int i=0; i<4; i++){
pthread_join(sleeppt[i], NULL);
}
}
Sample output (yours may vary):
message: 12345 and time: 0
message: 67890 and time: 3
message: abcde and time: 2
message: fghij and time: 1
shuffled message: 12345
shuffled message: fghij
shuffled message: abcde
shuffled message: 67890
Also, your split_message
array isn't big enough to hold the string you're copying in. You need a total of 6 bytes, for for each of 5 characters and one for the null terminator.
struct argsparam {
int sleep;
char split_message[6];
};
You're "lucky" that you're not stepping on memory you shouldn't due to structure padding.
Upvotes: 3