1

So I wrote this script:

def schedule_setup():
# KILL OLD THREADS SHOULD THEY EXIST
global active
active = False
time.sleep(3)
active = True
global threadlist
threadlist = []

try:
    sql = "SELECT TIME_TO_RUN FROM time_table"
    cursor.execute(sql)
    results = cursor.fetchall()
    for row in results:
        #row[0].strftime('%H:%M')
        t = threading.Thread(target=th,args=(row[0].strftime('%H:%M'),))
        t.start()
        threadlist.append(t)
    # JOIN all threads to main memory
    #for count in threadlist:
        #count.join()
    sql = "UPDATE motor SET UPDATE_SCHEDULE = 0"
    try:
        cursor.execute(sql)
        # commit the changes in database
        db.commit()
    except:
        # Rollback in case there is any error
        print "no worky"
        db.rollback()
except:
    print "Error: UNABLE TO GET TABLE DATA"

It takes times setup on sql and creates a scheduled event to do an action. I put in the beginning the thread "killer" for all active threads so that if I ever call this thread again because times have been updated it can kill old ones and replace them with the new. It all works how I want it to, but as soon as the action is called the whole program crashes... here is the code it calls:

def th(run_time):
    global active
    schedule.every().day.at(run_time).do(run_motor)

    while active == True:
        schedule.run_pending()
        time.sleep(1)

see how the thread checks every second? So threads are killed when I try to create new ones, but when "run_motor" gets called, afterwords the main program that is supposed to loop indefinitely crashes and sometimes other threads are still going so it is all very strange to me.

  • What does "crashes" mean, exactly? Do you get an error message? Do you get a traceback? If so, spell them out for us. – Tim Peters Oct 06 '13 at 20:53
  • BTW, since you say sometimes threads are still going, it's certain that your `time.sleep(3)` isn't long enough for all threads to end. There's not enough info here to guess why, though. You should be `.join()`ing threads if you want to be sure they're done - sleeping is never a reliable alternative. – Tim Peters Oct 06 '13 at 20:55
  • Would also help ;-) if you fixed the indentation in the code - unclear where, for example, `schedule_setup()` ends, since everything is flush to the left border. – Tim Peters Oct 06 '13 at 21:00
  • Thank you Tim, I will try to fix that. I looked more into the schedule library I am using and it turns out there IS a way to stop all timed processes. I think what I need to do is actually put the thread in a class, and setup all the scheduling and just run one schedule event... that way I can kill them all at once. I should probably have been more careful about studying the library before I asked this question but I really appreciate your input. :) – Craig Travis Oct 06 '13 at 21:36
  • - edit - no traceback. sometimes the whole thing completely shuts down so I think you are right, the time.sleep is not the best code to work with (I kind of knew that I really just wanted results before I improved code). – Craig Travis Oct 06 '13 at 21:38

1 Answers1

0

You can't sanely kill a thread. Instead of killing a thread, code the thread to do what, and only what, you want it to do. That way, you'll have no need to kill it.

If you share a car with two other people and the car isn't in the driveway, you can't just kill the person driving the car. That will free the car for your use, but it won't be in the driveway because you killed the driver before they had a chance to put it back. Instead, call them and tell them to bring the car back and let them release the car when they're done.

Community
  • 1
  • 1
David Schwartz
  • 179,497
  • 17
  • 214
  • 278
  • Right, I'm not directly killing a thread if you read I let the thread end on its own, and I use a time.sleep(3) to make sure I give the thread plenty of time to end itself properly. My problem is when the thread is doing the task, when it runs the task the thread should just keep going but it doesn't, the program crashes instead. – Craig Travis Oct 06 '13 at 20:55