1

I know this sounds a lot like this similarly-worded question, but there are differences, so bear with me.

I'm trying to create a reusable "Timer" class which calls a specified callback every N seconds, until you call stop. As inspiration, I used the link above, with a built-in event wrapped in a stop method. Here's how the basic class looks:

import time
import threading
from threading import Thread
from threading import Event

# Mostly inspired by https://stackoverflow.com/questions/12435211/python-threading-timer-repeat-function-every-n-seconds
class RepeatingTimer(Thread):
    def __init__(self, interval_seconds, callback):
        Thread.__init__(self)
        self.stop_event = Event()
        self.interval_seconds = interval_seconds
        self.callback = callback
        self.setDaemon(True)

    def start(self):
        while not self.stop_event.wait(self.interval_seconds):
            self.callback()
            time.sleep(0) # doesn't seem to do anything
    
    def stop(self):
        self.stop_event.set()

Looks good, even includes time.sleep(0) based on this question.

It doesn't do what I thought; the call to start never seems to return or yield, ever. Consider this use-case:

def print_status(message):
  print(message)

def print_r1():
  print_status("R1")

def print_r2():
  print_status("R2")

r1 = RepeatingTimer(1, print_r1)
r2 = RepeatingTimer(0.5, print_r2)

r1.start()
r2.start()

The call to r1.start never terminates. It continues on forever. The output on the console, after four seconds, is:

R1

R1

R1

R1

This prompted me to introduce the time.sleep(0) call, although that doesn't seem to do anything.

I also tried with and without self.setDaemon(True), but that also seems to have no effect.

I also tried converting this into two classes: one with just the event wrappers (a StoppableTimer class), and another that simply creates and recreates the StoppableTimer in the callback, but that doesn't work either. Here's what it looks like:

class StoppableTimer(Thread):
    def __init__(self, interval_seconds, callback):
        Thread.__init__(self)
        self.stop_event = Event()
        self.interval_seconds = interval_seconds
        self.callback = callback
        self.setDaemon(True)

    def start(self):
        time.sleep(self.interval_seconds)
        self.callback()
    
    def stop(self):
        self.stop_event.set()


class RepeatingTimer:
    def __init__(self, interval_seconds, callback):
        self.interval_seconds = interval_seconds
        self.callback = callback
        self.timer = StoppableTimer(interval_seconds, self.refresh_timer)

    def start(self):
        self.timer.start()

    def stop(self):
        self.timer.stop()

    def refresh_timer(self):
        self.stop()
        self.callback()
        self.timer = StoppableTimer(self.interval_seconds, self.refresh_timer)
        self.timer.start()

I'm completely at a loss on how to make this work. I'm also mostly a beginner to Python, so please add sufficient explanation to your answer so I can grasp what the fundamental issue is.

I also read a bit about the Global Interpreter Lock on SO, but I don't understand how that could be an issue.

For reference, I'm running Python 3.6.3 on Ubuntu 17.10

Community
  • 1
  • 1
nightblade9
  • 354
  • 2
  • 15
  • You seem to have overriden `Thread`'s `start` method, which, as you can imagine, is not a great idea, especially if you never call the actual Thread.start – pvg Nov 14 '17 at 18:23
  • @pvg this is probably my inexperience with Python here. I literally copy-pasted the code from the linked thread, and modified it to make the event a member variable instead of an externality. I'll make a note to edit the code sample in the linked question. – nightblade9 Nov 14 '17 at 21:17
  • You didn't, I checked :). You've rewritten the code so that it doesn't work. Someone would have noticed it doesn't work in the original answers and pointed it out. – pvg Nov 14 '17 at 21:38
  • @pvg I did copy-paste it, but at some point, I renamed `run` to `start` (without realizing the implications of it). That might explain the downvotes ... – nightblade9 Nov 15 '17 at 14:24

1 Answers1

3

Short answer :

Don't override start(). Override run() instead.

Long answer because you're asking for details :

With the class definition in your first snippet, you've created a class which inherits from Thread, however you've overriden the start() method supposed to start your thread by a new method which is looping until the stop_event is set, that is to say, the method supposed to actually start your thread doesn't do this anymore.

So, when you try to start your thread, you actually run the loop calling your callback function in your current and only thread. And since it's an infinite loop, your second "thread" is not started, and you have no way to "stop" it.

You mustn't override start (well not in this way). Instead, override the run method. This is the method that will be run by your thread when you start it.

Also, you should do super().__init__() instead of Thread.__init__(self). The first one is the proper way to call an inherited method in Python.

class RepeatingTimer(Thread):
    def __init__(self, interval_seconds, callback):
        super().__init__()
        self.stop_event = Event()
        self.interval_seconds = interval_seconds
        self.callback = callback

    def run(self):
        while not self.stop_event.wait(self.interval_seconds):
            self.callback()

    def stop(self):
        self.stop_event.set()

And with the functions you've defined you can do :

r1 = RepeatingTimer(1, print_r1)
r2 = RepeatingTimer(0.5, print_r2)

r1.start()
r2.start()
time.sleep(4)
r1.stop()
r2.stop()

Here is the relevant documentation for Thread.

Unatiel
  • 1,060
  • 1
  • 11
  • 17
  • 1
    A saner way to avoid all of this is 'don't inherit from Thread'. There's no obvious "is a" relationship between a thread and a timer. A timer might be implemented using a thread, it might contain a thread but it isn't a thread. – pvg Nov 14 '17 at 18:54
  • @pvg I don't have a full picture of what that would look like. Feel free to create a new answer and explain/illustrate your suggestion with some code. – nightblade9 Nov 14 '17 at 21:27
  • Thanks for the working code and detailed explanation (+1). I'm going to wait and see if someone can suggest how to do this without inheriting from `Thread`, and if nothing happens in a couple of days, I'll accept this answer. – nightblade9 Nov 14 '17 at 21:33
  • @nightblade9 I don't think there's much to be gained in my writing code for you for a different question. You should accept this answer - it answers your question, which itself is really mostly a 'typo or no longer reproducible' type of problem that's unlikely to help others. What you _should_ do is follow the suggestion to read the documentation on Thread - it covers the subclassing and delegation cases right away and explicitly tells you not to override methods other than `run`. You can also review a threading tutorial such as https://pymotw.com/2/threading/ that covers both cases as well. – pvg Nov 14 '17 at 23:17
  • @nightblade9 Also when you're comfortable with Python, you may be interested in asyncio. For instance this : https://stackoverflow.com/questions/37512182/how-can-i-periodically-execute-a-function-with-asyncio – Unatiel Nov 14 '17 at 23:32