Skip to content

Conversation

@leonardmq
Copy link
Collaborator

What does this PR do?

Local PDF processing sometimes fails - seems to be hardware dependent - and when that happens, the whole process pool that converts pages into images crashes.

Fix:

  • recreate the process pool if any process crashes

Checklists

  • Tests have been run locally and passed
  • New tests have been added to any work in /lib

@leonardmq leonardmq marked this pull request as draft December 16, 2025 06:24
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 16, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Dec 16, 2025

📊 Coverage Report

Overall Coverage: 91%

Diff: origin/main...HEAD

  • libs/core/kiln_ai/utils/pdf_utils.py (34.8%): Missing lines 25-30,98-100,105,108-110,116-117

Summary

  • Total: 23 lines
  • Missing: 15 lines
  • Coverage: 34%

Line-by-line

View line-by-line diff coverage

libs/core/kiln_ai/utils/pdf_utils.py

Lines 21-34

  21 
  22 def _reset_pdf_conversion_executor():
  23     """Reset the PDF conversion executor, shutting down the old one if it exists."""
  24     global _pdf_conversion_executor
! 25     if _pdf_conversion_executor is not None:
! 26         try:
! 27             _pdf_conversion_executor.shutdown(wait=False)
! 28         except Exception:
! 29             pass
! 30         _pdf_conversion_executor = None
  31 
  32 
  33 # Lazy load for speed, singleton so dev-server reloading doesn't recreate the executor
  34 def get_pdf_conversion_executor() -> ProcessPoolExecutor:

Lines 94-114

   94             pdf_path,
   95             output_dir,
   96         )
   97         return result
!  98     except (BrokenExecutor, RuntimeError) as e:
!  99         error_msg = str(e).lower()
! 100         if (
  101             "process pool" in error_msg
  102             or "child process" in error_msg
  103             or isinstance(e, BrokenExecutor)
  104         ):
! 105             logger.warning(
  106                 f"Process pool became unusable, recreating executor. Error: {e}"
  107             )
! 108             _reset_pdf_conversion_executor()
! 109             executor = get_pdf_conversion_executor()
! 110             result = await loop.run_in_executor(
  111                 executor,
  112                 _convert_pdf_to_images_sync,
  113                 pdf_path,
  114                 output_dir,

Lines 112-121

  112                 _convert_pdf_to_images_sync,
  113                 pdf_path,
  114                 output_dir,
  115             )
! 116             return result
! 117         raise
  118 
  119 
  120 def _shutdown_pdf_conversion_executor():
  121     """Shutdown the PDF conversion executor process."""


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants