Simple thread runs forever until it's killed

If you are using the main C++ distribution of wxWidgets, Feel free to ask any question related to wxWidgets development here. This means questions regarding to C++ and wxWidgets, not compile problems.
Post Reply
Virchanza
I live to help wx-kind
I live to help wx-kind
Posts: 172
Joined: Sun Jul 19, 2009 6:12 am

Simple thread runs forever until it's killed

Post by Virchanza »

You might see some really bad programming errors in this post, but please humour me and read all the way to end before beginning to compose your reply.

Let's say we have a thread that makes the PC speaker beep periodically (i.e. beep beep beep). This thread is extremely simple:

Code: Select all

class BeeperThread : public wxThread {
public:

    void *Entry()
    {
        for (;;)
        {
            PC_Speaker_Set_ON();
            this->Sleep(300);      /* Stay on for 300 milliseconds */

            PC_Speaker_Set_OFF();
            this->Sleep(1000);     /* Stay off for 1 second */
        }
    }
};
In the program's main thread (i.e. the thread that controls the GUI), there is a dialog box, and on this dialog box there is a button that says "Turn On Beeper". If you click this button, the Beeper gets turned on, and then the text on the button changes to "Turn Off Beeper".

After having started the Beeper, if you click the button again, the Beeper gets turned off, and then the text on the button changes back to "Turn On Beeper".

Here's the code in the main thread for the clicking of the button:

Code: Select all

void DialogMain::OnClickButton_ToggleBeeper(wxCommandEvent &event)
{
    static BeeperThread *p_beeper_thread = 0;

    if ( !p_beeper_thread ) /* To check if they clicked "Turn On" or "Turn off" */
    {
        /* They clicked "Turn On" */

        p_beeper_thread = new Beeper_Thread;

        if ( p_beeper_thread->Create() != wxTHREAD_NO_ERROR )
        {
            wxMessageBox(wxT("Thread Creation Failed"));

            delete p_beeper_thread;
            p_beeper_thread = 0;

            return;
        }
        
        p_beeper_thread->SetPriority(WXTHREAD_MIN_PRIORITY);

        if ( p_beeper_thread->Run() != wxTHREAD_NO_ERROR )
        {
            wxMessageBox(wxT("Thread Run Failed"));

            delete p_beeper_thread;
            p_beeper_thread = 0;

            return;
        }

        /* If we reach here, everything went off without a hitch
           and the beeper is running */

        this->m_button_ToggleBeeper->SetLabel(wxT("Turn Off Beeper"));
    }
    else
    {
        /* They clicked "Turn Off" */

        p_beeper_thread->Kill();

        PC_Speaker_Set_OFF(); /* In case the PC Speaker was ON when the thread was killed */

        delete p_beeper_thread;
        p_beeper_thread = 0;

        this->m_button_ToggleBeeper->SetLabel(wxT("Turn On Beeper"));
    }
}

OK, I have questions.

1) If you look at the Entry function for my BeeperThread, you'll see that the thread just runs in an eternal loop and it never checks TestDestroy. Is it OK to write a thread like this in wxWidgets? Basically I want the thread to run forever until the Main thread kills it.

2) If you look at the code for when the user clicks "Turn Off Beeper", you'll see that I call the Kill member function on my Beeper_Thread object. Is this OK? I've heard that the Kill function can have bad consequences such as leaving the C Standard Library in an undefined state.

I realise that there is a simple solution to this problem. Within the "Entry" function, all I have to do is call the TestDestroy function upon each iteration of the loop. Then, from the main GUI thread, all I have to do is call the Delete member function on my Beeper Thread object.

So why don't I just do that? Well the thing is, I'm not working with a PC speaker at all -- I just gave that example because it's really simple and it gets my point across.

In actuality, the secondary thread I have running is an Ethernet network sniffer. It sniffs Ethernet frames from the network in an eternal loop as follows:

Code: Select all

void *Entry()
{
    for (;;)
    {
        FrameInfo fi; /* Data type to store an Ethernet frame */

        Recieve_Ethernet_Frame( &fi ); /* This reads a frame from the network */

        Process_Ethernet_Frame( &fi ); /* This processes the Ethernet frame */
    }
}
Now there's just one little problem with the above code snippet. The function, Receive_Ethernet_Frame is synchronous, meaning that it won't return until it actually pulls an Ethernet frame from the network.

It's possible that this function could take 5 seconds, or 10 seconds, or 10 minutes to return if there's no traffic on the network!

If I were to edit the "eternal loop" above, and add a call to TestDestroy, then I would have the problem of the thread lingering... and lingering... and lingering... until the call to Receive_Ethernet_Frame returns. Only then would I be able to call TestDestroy and finally stop the thread.

So my situation can be summed up as:
1) I have a secondary thread which contains an eternal loop.
2) Inside this eternal loop, there's a call to a synchronous function which could take seconds or minutes to return.
3) The challenge is to be able to (safely) terminate this thread instantly from within the Main thread (by "instantly" I mean within 2 seconds or so).

So... any ideas?

Thanks a lot for your time if you read this far, I appreciate it.
TrV
Ultimate wxWidgets Guru
Ultimate wxWidgets Guru
Posts: 630
Joined: Wed Jul 04, 2007 1:12 pm

Re: Simple thread runs forever until it's killed

Post by TrV »

Virchanza wrote: 1) If you look at the Entry function for my BeeperThread, you'll see that the thread just runs in an eternal loop and it never checks TestDestroy. Is it OK to write a thread like this in wxWidgets? [...]

2) If you look at the code for when the user clicks "Turn Off Beeper", you'll see that I call the Kill member function on my Beeper_Thread object. Is this OK? I've heard that the Kill function can have bad consequences such as leaving the C Standard Library in an undefined state.
OK: definitely not! As this is OOP, this is OOPolitically correct to ask the thread to kill itself rather than another object to kill it (suicide is allowed, not murder ;))
Virchanza wrote:I realise that there is a simple solution to this problem. Within the "Entry" function, all I have to do is call the TestDestroy function upon each iteration of the loop. Then, from the main GUI thread, all I have to do is call the Delete member function on my Beeper Thread object.
This sure is the best design solution. But...
Virchanza wrote:[...] The function, Receive_Ethernet_Frame is synchronous, meaning that it won't return until it actually pulls an Ethernet frame from the network.

It's possible that this function could take 5 seconds, or 10 seconds, or 10 minutes to return if there's no traffic on the network!

[...]

So my situation can be summed up as:
1) I have a secondary thread which contains an eternal loop.
2) Inside this eternal loop, there's a call to a synchronous function which could take seconds or minutes to return.
3) The challenge is to be able to (safely) terminate this thread instantly from within the Main thread (by "instantly" I mean within 2 seconds or so).
- Besides the fact Kill() is bad programing, i think it's quite ok if no share resource is used by the thread going to be killed. As for the thread resources, you can expect the OS to do the dirty work for you and free what has been allocated when the app terminates.

- Can't you handle some kind of timeout processing into "Receive_Ethernet_Frame"? What is inside this procedure? Generally all sockets process are timeout-ed. If you haven't set one, some default values set by the OS is used.
Can't you modify the code to do so?
Virchanza
I live to help wx-kind
I live to help wx-kind
Posts: 172
Joined: Sun Jul 19, 2009 6:12 am

Post by Virchanza »

Thanks a lot for your reply.

It actually wouldn't be impossible for me to do one of the following:
a) Call an asynchronous function instead
or
b) Call a "semi-synchronous" function that is synchronous until it times out and returns.

But I really don't want to do either of those because my code is so wonderfully simple right now. I really like the idea of the Main thread being able to simply kill the secondary thread instantly.

If I were to call an asynchronous function instead, then my code might be something like:

Code: Select all

void *Entry()
{
    for (;;)
    {
        FrameInfo fi; /* Data type to store an Ethernet frame */

        while (!ASYNC_Recieve_Ethernet_Frame( &fi ))
        {
            this->Sleep(1);
        }

        Process_Ethernet_Frame( &fi ); /* This processes the Ethernet frame */
    }
}
If I do choose to call an asynchronous function instead, I think there's a higher chance of me missing frames on the network. You see, one of the things my program does is send a flood of ARP requests to all possible IP addresses on the network, so if there's a big network of PC's then there could be a massive flood of replies in a very short space of time. I was thinking that a synchronous function would be better for catching as many replies as possible. If I was to call an asynchronous function and pause for 5 milliseconds between attempts, I think I might miss some replies. Of course I could remove the 5-millisecond delay, but then the thread might hog the CPU as it constantly calls the asynchronous function hoping that there's an Ethernet frame sitting there waiting to be plucked.

Do I definitely need to find an alternative to the synchronous function if I want the thread to always terminate in less than 2 seconds? Is there no way at all that I can make the Main thread stop the secondary thread dead in its tracks?

I've been thinking about one other thing. I could cause the synchronous function to immediately return by sending an Ethernet frame onto the network all by myself. It's an option.

I appreciate any further advice you have.
Post Reply