threads and callbacks

[steve1_rm]

Hello,

I am wondering where I should call my pthread_join().

I have a file called stopwatch.c. In that that file I create a thread and in that thread I pass a function that will sleep for a period of seconds. When the seconds have completed it will call a call back function.

I would like to implement this file in any of my programs. However, I would like the call back to be called in the main function, and not in the stopwatch_timer.c.

The reason this is, is because the main.c has some data that needs to be processed when the time expires. And the stopwatch_timer shouldn't have access this to this data.

Just one more question, am I passing my seconds correctly, as I seem to have incorrect data been sent. This is what I get.
=== g_start_timer(): Seconds: 7643124

Many thanks,

stopwatch_timer.c

/* Implemenation for using a stopwatch timer */
#include <stdio.h>
#include <pthread.h>

/* function prototypes */
static void* g_start_timer(void *args);
void timeout_cb();

/* call back function that is called when the time has expired */
void timeout_cb()
{
    printf("=== Time has expired ===\n");
}

/* start the stop watch in its own thread */
void start_stopwatch(int seconds)
{
    pthread_t thread_id;
    int rc = 0;
    int secs = 3;

    printf("=== start_stopwatch(): %d\n", seconds);

    rc = pthread_create(&thread_id, NULL, g_start_timer, (void *) &secs);

    if(rc)
        printf("Error creating thread\n");
    else
        printf("=== Success in creating thread\n");
}

/* Start the timer in the thread */
static void* g_start_timer(void *args)

{
    /* get the number of seconds and cast to int */
    int seconds = *((int*) args);
    printf("=== g_start_timer(): Seconds: %d\n", seconds
);

    /* call back function */
    void (*function_ptr)();

    /* assign the address of the cb to the function pointer */
    function_ptr = timeout_cb;

    /* sleep for a period of seconds specified */
    sleep(3);

    /* call the call back function after time expired */
    (*function_ptr)();

    pthread_exit(NULL);
}

[Salem]

> rc = pthread_create(&thread_id, NULL, g_start_timer, (void *) &secs);
You're passing a pointer to a local which no longer exits by the time the thread actually runs.

[Codeplug]

>> (void *) &secs
You can't pass a pointer to stack variable. start_stopwatch() could return before the thread starts. You can use the pointer value itself to pass integral values - (void*)secs. But don't you want to also pass the callback function to the thread as well?

Using a single thread for a single timed callback is a bit overkill and doesn't scale well. But it's good for fun

gg

[steve1_rm]

Hello,

This program does work. I call start_stopwatch. And it will callback in main.c timeout_cb() after the time has expired. I also run this in a seperate thread, as I don't want to block in main, as I will have other code I need to run.

1) The seconds in g_start_timer always gives rubbish. I thought I might have solved this by creating the structure on the heap. Is there anyway I can solve this. I was thinking of creating the seconds element on the heap. But think this is over kill

2) This program works fine, but if I comment out the line in main printf("=== timeout_cb: %p\n", timeout_cb); it will stack dump.

3) when is the best time to free the memory. I was freeing it in main. But I am worried if the memory is freed before the thread has finished. That could cause a very unexpected result. I am thinking I could use thread_join() as free the memory after this call. However, I would need to return the thead_id that is created in the stop_watch.c, is there a way to return the thread_id that is created in stop_watch.c

Many thanks for any suggestions,

main.c

/* main.c */
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>

#include "stop_watch.h"

/* call this when the time expires */
void timeout_cb()
{
    printf("=== your time is up run some job here ===\n");
}

int main()
{
    struct data_struct *g_data_struct =
        (struct data_struct*) calloc(1, sizeof(*g_data_struct));

    if(!g_data_struct)
    {
        printf("=== failed to allocate memory ===\n");
        return 0;
    }
    
    g_data_struct->seconds = 3;
    g_data_struct->func_ptr = timeout_cb;

    //  printf("=== timeout_cb: %p\n", timeout_cb);

    start_stopwatch(g_data_struct);

    // free(g_data_struct);
    printf("=== End of Program - all threads in ===\n");

    pthread_exit(NULL);

    return 0;
}

stop_watch.h

/* stop_watch.h */
struct data_struct
{
    int seconds;
    void (*func_ptr)(void);
};
void start_stopwatch(struct data_struct *g_data_struct);

stop_watch.c

#include <stdio.h>
#include <pthread.h>

#include "stop_watch.h"

static void* g_start_timer(void *args)
{
    void (*function_pointer)();

    int seconds = ((struct data_struct*) args)->seconds;
    function_pointer = ((struct data_struct*) args)->func_ptr;

    printf("=== go to sleep for %d\n", seconds);

    sleep(seconds);

    (void) (*function_pointer)();

    pthread_exit(NULL);

    return 0;
}

void start_stopwatch(struct data_struct *g_data_struct)
{
    pthread_t thread_id;
    int rc;

    int seconds = g_data_struct->seconds;
    printf("=== start_stopwatch(): %d\n", seconds);

    rc =  pthread_create(&thread_id, NULL, g_start_timer, (void *) &g_data_struct);

    if(rc)
        printf("=== Failed to create thread\n");
}

[Salem]

> This program does work .... The seconds in g_start_timer always gives rubbish
What planet are you on?

Your program may superficially work some of the time in this simple exercise, but that doesn't remove the very serious problems it has when it comes to referring to memory which no longer exists.

Even if it ran perfectly every time, it would still be broken.

It runs on pure dumb luck, that's all. Not because of your skill as a programmer.

Very simple programs often work despite your best attempts to make a mess of it. It's only when you scale up to large programs that you find out whether your technique is built on rock or on sand.

We've told you some of the things to fix, but you haven't.