Skip to content

Commit 2093ad3

Browse files
authored
fix(docx): add timeout and isolated profile to LibreOffice conversions (#3820)
* fix(docx): add timeout and isolated profile to LibreOffice conversions The shared LibreOffice->PDF converter used by the DOCX and PPTX backends ran soffice with no timeout, so a hung conversion blocked the calling thread forever. The XLSX backend duplicated the same subprocess call with its own 60s timeout, but neither passed -env:UserInstallation, so concurrent conversions collided on LibreOffice's default profile lock and failed intermittently and silently. Give every LibreOffice invocation a bounded timeout and its own throwaway profile directory, and drop the XLSX backend's duplicated logic in favor of the shared helper. Resolves #3819 Signed-off-by: Daniel Nguyen <danielnguyenh07@gmail.com> * fix(docx): isolate LibreOffice profile in convert_to_modern_format too convert_with_libreoffice already had -env:UserInstallation profile isolation from the previous commit, but convert_to_modern_format (used for legacy .doc/.xls/.ppt -> modern Open XML conversion) still shared the default LibreOffice profile, so it remained vulnerable to the same concurrent-conversion lock contention. Extract the isolated-profile setup into a shared _isolated_libreoffice_profile context manager so both conversion paths use it instead of duplicating the logic a third time. Also add type annotations to convert_with_libreoffice and move the previous inline comments into docstrings per review feedback. Signed-off-by: Daniel Nguyen <danielnguyenh07@gmail.com> --------- Signed-off-by: Daniel Nguyen <danielnguyenh07@gmail.com>
1 parent 873f990 commit 2093ad3

3 files changed

Lines changed: 204 additions & 64 deletions

File tree

docling/backend/docx/drawingml/utils.py

Lines changed: 80 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,26 @@
33
import os
44
import shutil
55
import subprocess
6+
from collections.abc import Iterator
7+
from contextlib import contextmanager
68
from io import BytesIO
79
from pathlib import Path
810
from tempfile import mkdtemp
9-
from typing import TYPE_CHECKING, Callable, Optional
11+
from typing import TYPE_CHECKING, Callable, Final, Optional
1012

1113
import pypdfium2
1214
from PIL import Image, ImageChops
1315

1416
if TYPE_CHECKING:
1517
from docx.document import Document
1618

19+
LIBREOFFICE_TIMEOUT_S: Final[int] = 60
20+
"""Maximum seconds to wait for a single LibreOffice conversion.
21+
22+
Without this, a hung ``soffice`` process (e.g. a modal dialog it can't
23+
show in headless mode) blocks the calling thread forever.
24+
"""
25+
1726

1827
def get_libreoffice_cmd(raise_if_unavailable: bool = False) -> Optional[str]:
1928
"""Return the libreoffice cmd and optionally test it."""
@@ -46,6 +55,27 @@ def get_libreoffice_cmd(raise_if_unavailable: bool = False) -> Optional[str]:
4655
return libreoffice_cmd
4756

4857

58+
@contextmanager
59+
def _isolated_libreoffice_profile() -> Iterator[str]:
60+
"""Yield a ``-env:UserInstallation`` argument backed by a throwaway profile.
61+
62+
LibreOffice takes an exclusive lock on its user profile directory.
63+
Without this, concurrent conversions (parallel workers, docling-serve
64+
handling simultaneous requests) share the default profile and collide
65+
on that lock, causing conversions to fail intermittently and silently.
66+
67+
Yields:
68+
A ``-env:UserInstallation=<uri>`` CLI argument pointing at a freshly
69+
created, empty profile directory. The directory is removed again
70+
once the ``with`` block exits.
71+
"""
72+
profile_dir = Path(mkdtemp(prefix="docling_lo_profile_"))
73+
try:
74+
yield f"-env:UserInstallation={profile_dir.as_uri()}"
75+
finally:
76+
shutil.rmtree(profile_dir, ignore_errors=True)
77+
78+
4979
def convert_to_modern_format(
5080
source: BytesIO | Path,
5181
source_suffix: str,
@@ -93,21 +123,23 @@ def convert_to_modern_format(
93123
else:
94124
input_path = source
95125

96-
subprocess.run(
97-
[
98-
libreoffice_cmd,
99-
"--headless",
100-
"--convert-to",
101-
target_suffix,
102-
"--outdir",
103-
str(tmp_dir),
104-
str(input_path),
105-
],
106-
stdout=subprocess.DEVNULL,
107-
stderr=subprocess.DEVNULL,
108-
check=True,
109-
timeout=timeout_s,
110-
)
126+
with _isolated_libreoffice_profile() as profile_arg:
127+
subprocess.run(
128+
[
129+
libreoffice_cmd,
130+
profile_arg,
131+
"--headless",
132+
"--convert-to",
133+
target_suffix,
134+
"--outdir",
135+
str(tmp_dir),
136+
str(input_path),
137+
],
138+
stdout=subprocess.DEVNULL,
139+
stderr=subprocess.DEVNULL,
140+
check=True,
141+
timeout=timeout_s,
142+
)
111143

112144
converted_path = tmp_dir / (input_path.stem + "." + target_suffix)
113145
if not converted_path.exists():
@@ -132,21 +164,38 @@ def get_docx_to_pdf_converter() -> Optional[Callable]:
132164

133165
if libreoffice_cmd:
134166

135-
def convert_with_libreoffice(input_path, output_path):
136-
subprocess.run(
137-
[
138-
libreoffice_cmd,
139-
"--headless",
140-
"--convert-to",
141-
"pdf",
142-
"--outdir",
143-
os.path.dirname(output_path),
144-
input_path,
145-
],
146-
stdout=subprocess.DEVNULL,
147-
stderr=subprocess.DEVNULL,
148-
check=True,
149-
)
167+
def convert_with_libreoffice(
168+
input_path: str | Path, output_path: str | Path
169+
) -> None:
170+
"""Convert a DOCX/PPTX file to PDF via LibreOffice.
171+
172+
Runs the conversion in its own throwaway LibreOffice profile
173+
(see ``_isolated_libreoffice_profile``) with a bounded timeout,
174+
so a hung ``soffice`` process cannot block the calling thread
175+
forever and concurrent conversions cannot collide on the
176+
default profile lock.
177+
178+
Args:
179+
input_path: Path to the source file to convert.
180+
output_path: Desired path for the converted PDF.
181+
"""
182+
with _isolated_libreoffice_profile() as profile_arg:
183+
subprocess.run(
184+
[
185+
libreoffice_cmd,
186+
profile_arg,
187+
"--headless",
188+
"--convert-to",
189+
"pdf",
190+
"--outdir",
191+
os.path.dirname(output_path),
192+
input_path,
193+
],
194+
stdout=subprocess.DEVNULL,
195+
stderr=subprocess.DEVNULL,
196+
check=True,
197+
timeout=LIBREOFFICE_TIMEOUT_S,
198+
)
150199

151200
expected_output = os.path.join(
152201
os.path.dirname(output_path),

docling/backend/msexcel_backend.py

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import logging
55
import posixpath
66
import shutil
7-
import subprocess
87
import warnings
98
from copy import deepcopy
109
from datetime import datetime
@@ -49,7 +48,7 @@
4948
from docling.backend.docx.drawingml.utils import (
5049
convert_to_modern_format,
5150
crop_whitespace,
52-
get_libreoffice_cmd,
51+
get_docx_to_pdf_converter,
5352
)
5453
from docling.datamodel.backend_options import MsExcelBackendOptions
5554
from docling.datamodel.base_models import FormatToMimeType, InputFormat
@@ -272,10 +271,6 @@ class MsExcelDocumentBackend(DeclarativeDocumentBackend, PaginatedDocumentBacken
272271
parsing, so threaded comments are not available for that format.
273272
"""
274273

275-
# Maximum seconds to wait for a single LibreOffice EMF/WMF conversion.
276-
# Raise this value if conversions time out on unusually large or complex files.
277-
LIBREOFFICE_TIMEOUT_S: Final[int] = 60
278-
279274
# Maximum uncompressed byte sizes accepted when reading members from the XLSX zip.
280275
# These caps guard against decompression-bomb payloads in drawing XML / image files.
281276
_MAX_DRAWING_BYTES: Final[int] = 10 * 1024 * 1024 # 10 MB
@@ -1147,36 +1142,11 @@ def _get_libreoffice_converter(self) -> Callable | None:
11471142
return self.xlsx_to_pdf_converter
11481143

11491144
self.xlsx_to_pdf_converter_init = True
1150-
libreoffice_cmd = get_libreoffice_cmd()
1151-
if libreoffice_cmd is None:
1145+
self.xlsx_to_pdf_converter = get_docx_to_pdf_converter()
1146+
if self.xlsx_to_pdf_converter is None:
11521147
_log.debug(
11531148
"LibreOffice not found — EMF/WMF images in XLSX will be skipped."
11541149
)
1155-
self.xlsx_to_pdf_converter = None
1156-
return None
1157-
1158-
def _convert(input_path: Path, output_path: Path) -> None:
1159-
subprocess.run(
1160-
[
1161-
libreoffice_cmd,
1162-
"--headless",
1163-
"--convert-to",
1164-
"pdf",
1165-
"--outdir",
1166-
str(output_path.parent),
1167-
str(input_path),
1168-
],
1169-
stdout=subprocess.DEVNULL,
1170-
stderr=subprocess.DEVNULL,
1171-
check=True,
1172-
timeout=self.LIBREOFFICE_TIMEOUT_S,
1173-
)
1174-
# LibreOffice names the output after the input stem
1175-
expected = output_path.parent / (input_path.stem + ".pdf")
1176-
if expected != output_path:
1177-
expected.rename(output_path)
1178-
1179-
self.xlsx_to_pdf_converter = _convert
11801150
return self.xlsx_to_pdf_converter
11811151

11821152
def _convert_emf_to_pil(self, image_bytes: bytes) -> PILImage.Image | None:

tests/test_drawingml_utils.py

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
import subprocess
2+
from io import BytesIO
3+
from pathlib import Path
4+
from unittest.mock import MagicMock
5+
6+
import pytest
7+
8+
from docling.backend.docx.drawingml import utils as drawingml_utils
9+
10+
11+
def _track_mkdtemp(monkeypatch) -> list[Path]:
12+
created_dirs: list[Path] = []
13+
real_mkdtemp = drawingml_utils.mkdtemp
14+
15+
def tracking_mkdtemp(*args, **kwargs):
16+
path = real_mkdtemp(*args, **kwargs)
17+
created_dirs.append(Path(path))
18+
return path
19+
20+
monkeypatch.setattr(drawingml_utils, "mkdtemp", tracking_mkdtemp)
21+
return created_dirs
22+
23+
24+
def test_convert_with_libreoffice_uses_timeout_and_isolated_profile(
25+
monkeypatch, tmp_path
26+
):
27+
monkeypatch.setattr(
28+
drawingml_utils, "get_libreoffice_cmd", lambda: "/usr/bin/soffice"
29+
)
30+
created_profile_dirs = _track_mkdtemp(monkeypatch)
31+
32+
captured_args: list[str] = []
33+
captured_kwargs: dict = {}
34+
35+
def fake_run(args, **kwargs):
36+
captured_args.extend(args)
37+
captured_kwargs.update(kwargs)
38+
# The isolated profile dir must exist while the "conversion" runs.
39+
assert created_profile_dirs[-1].exists()
40+
# Mirror LibreOffice's real behavior of writing the PDF next to the
41+
# input, named after the input's stem.
42+
(tmp_path / "drawing_only.pdf").write_bytes(b"%PDF-1.4")
43+
return MagicMock(returncode=0)
44+
45+
monkeypatch.setattr(drawingml_utils.subprocess, "run", fake_run)
46+
47+
converter = drawingml_utils.get_docx_to_pdf_converter()
48+
assert converter is not None
49+
50+
input_path = tmp_path / "drawing_only.docx"
51+
output_path = tmp_path / "drawing_only.pdf"
52+
input_path.write_bytes(b"")
53+
54+
converter(input_path, output_path)
55+
56+
assert captured_kwargs["timeout"] == drawingml_utils.LIBREOFFICE_TIMEOUT_S
57+
58+
profile_flag = next(
59+
a for a in captured_args if str(a).startswith("-env:UserInstallation=")
60+
)
61+
assert created_profile_dirs[-1].as_uri() in profile_flag
62+
63+
# The isolated profile directory is cleaned up after the call.
64+
assert not created_profile_dirs[-1].exists()
65+
66+
67+
def test_convert_with_libreoffice_cleans_up_profile_on_timeout(monkeypatch, tmp_path):
68+
monkeypatch.setattr(
69+
drawingml_utils, "get_libreoffice_cmd", lambda: "/usr/bin/soffice"
70+
)
71+
created_profile_dirs = _track_mkdtemp(monkeypatch)
72+
73+
def fake_run(args, **kwargs):
74+
raise subprocess.TimeoutExpired(cmd=args, timeout=kwargs.get("timeout"))
75+
76+
monkeypatch.setattr(drawingml_utils.subprocess, "run", fake_run)
77+
78+
converter = drawingml_utils.get_docx_to_pdf_converter()
79+
assert converter is not None
80+
81+
input_path = tmp_path / "drawing_only.docx"
82+
output_path = tmp_path / "drawing_only.pdf"
83+
input_path.write_bytes(b"")
84+
85+
with pytest.raises(subprocess.TimeoutExpired):
86+
converter(input_path, output_path)
87+
88+
# A hung/killed conversion must not leak its profile directory.
89+
assert not created_profile_dirs[-1].exists()
90+
91+
92+
def test_convert_to_modern_format_uses_isolated_profile(monkeypatch):
93+
monkeypatch.setattr(
94+
drawingml_utils, "get_libreoffice_cmd", lambda: "/usr/bin/soffice"
95+
)
96+
created_profile_dirs = _track_mkdtemp(monkeypatch)
97+
98+
captured_kwargs: dict = {}
99+
100+
def fake_run(args, **kwargs):
101+
captured_kwargs.update(kwargs)
102+
# The isolated profile dir (most recently created) must exist while
103+
# the "conversion" runs, separate from the outer working tmp_dir.
104+
assert created_profile_dirs[-1].exists()
105+
outdir = Path(args[args.index("--outdir") + 1])
106+
(outdir / "input.docx").write_bytes(b"PK\x03\x04")
107+
return MagicMock(returncode=0)
108+
109+
monkeypatch.setattr(drawingml_utils.subprocess, "run", fake_run)
110+
111+
result = drawingml_utils.convert_to_modern_format(
112+
BytesIO(b"legacy doc bytes"), "doc", "docx", timeout_s=5
113+
)
114+
115+
assert isinstance(result, BytesIO)
116+
assert captured_kwargs["timeout"] == 5
117+
118+
# Both the outer working directory and the isolated profile directory
119+
# are cleaned up once the conversion completes.
120+
assert not created_profile_dirs[0].exists()
121+
assert not created_profile_dirs[-1].exists()

0 commit comments

Comments
 (0)