Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## Version 0.26.2

* Force-exit sync workers after hard timeouts so interpreter shutdown cannot leave them waiting for task-created threads ([#384](https://github.com/closeio/tasktiger/pull/384)).

## Version 0.26.1

* `Task.tasks_from_queue` leaves out tasks without a payload if `include_not_found` is false. Versions 0.23.0 to 0.26.0 raised `UnboundLocalError` or returned the previous task again ([#382](https://github.com/closeio/tasktiger/pull/382)).
Expand Down
2 changes: 1 addition & 1 deletion tasktiger/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from .tasktiger import TaskTiger, run_worker
from .worker import Worker

__version__ = "0.26.1"
__version__ = "0.26.2"
__all__ = [
"TaskTiger",
"Worker",
Expand Down
10 changes: 6 additions & 4 deletions tasktiger/worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import os
import random
import signal
import sys
import time
import uuid
from collections import OrderedDict
Expand Down Expand Up @@ -832,10 +831,13 @@ def _mark_done() -> None:
runner = runner_class(self.tiger)
runner.on_permanent_error(task, execution)

# Exit the process with an error code if a task timed out to
# prevent any inconsistent state in case the runner requires it.
# Force the process to exit after a task times out. A normal interpreter
# shutdown can wait indefinitely for threads started by the task.
if self.executor.exit_worker_on_job_timeout and has_job_timeout:
sys.exit("exiting worker due to job timeout error")
try:
os.write(2, b"exiting worker due to job timeout error\n")
finally:
os._exit(1)

def _worker_run(self) -> None:
"""
Expand Down
10 changes: 10 additions & 0 deletions tests/tasks.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import threading
import time
from math import ceil

Expand Down Expand Up @@ -54,6 +55,15 @@ def long_task_killed():
time.sleep(DELAY * 2)


@tiger.task(hard_timeout=DELAY)
def long_task_with_blocked_thread():
blocker = threading.Event()
# A non-daemon thread makes normal interpreter shutdown wait indefinitely.
thread = threading.Thread(target=blocker.wait, daemon=False)
thread.start()
time.sleep(DELAY * 2)


@tiger.task(hard_timeout=DELAY * 2)
def long_task_ok():
# Signal task has started
Expand Down
21 changes: 14 additions & 7 deletions tests/test_workers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import time
from multiprocessing import Process

import pytest
from freezefrog import FreezeTime

from tasktiger import Task, Worker
Expand All @@ -15,8 +14,8 @@
from .config import DELAY
from .tasks import (
exception_task,
long_task_killed,
long_task_ok,
long_task_with_blocked_thread,
simple_task,
sleep_task,
system_exit_task,
Expand Down Expand Up @@ -182,11 +181,19 @@ def test_handles_exception(self, tiger, ensure_queues):
ensure_queues(error={"default": 1})

def test_handles_timeout(self, tiger, ensure_queues):
Task(tiger, long_task_killed).delay()
worker = Worker(tiger, executor_class=SyncExecutor)
# Worker should exit to avoid any inconsistencies.
with pytest.raises(SystemExit):
worker.run(once=True, force_once=True)
Task(tiger, long_task_with_blocked_thread).delay()
worker = Process(
target=external_worker,
kwargs={"worker_kwargs": {"executor_class": SyncExecutor}},
)
worker.start()
worker.join(timeout=DELAY * 5)
try:
assert worker.exitcode == 1
finally:
if worker.is_alive():
worker.kill()
worker.join()
ensure_queues(error={"default": 1})

def test_heartbeat(self, tiger):
Expand Down