Skip to content

Commit 2266020

Browse files
guitargeekguitargeek
committed
[gpad] Fix crash in streaming web canvas after TColor::DefinedColors(1)
TColor::DefinedColors(1) sets the global color bookkeeping to the sticky "always store colors" mode (gLastDefinedColors = -1). When a canvas is then serialized to JSON for the web display - as JupyROOT does to show a canvas inline in a notebook - TWebCanvas::CreatePadSnapshot temporarily detaches the pad primitives (fPrimitives = nullptr) and calls the no-arg TColor::DefinedColors() to clear the change flag so the canvas streamer skips color storage. In sticky mode that call returns kTRUE early without resetting the flag, so TCanvas::Streamer entered the color-storing branch and dereferenced the null fPrimitives, crashing with a segmentation violation. Guard the color-storing branch with a null check on fPrimitives. Colors and the palette are delivered separately by TWebCanvas::AddColorsPalette in that path, and normal file saving (where fPrimitives is never null) is unaffected. Add a regression test that reproduces the original report by drawing a canvas and calling TWebCanvas::CreateCanvasJSON after TColor::DefinedColors(1); it segfaults without this fix and passes with it. The JupyROOT notebook test is registered without output comparison (it draws a canvas, so its output is not reproducible), matching Cpp_IMT_Canvas.ipynb. Closes #20018. 🤖 Done with the help of [Claude Code](https://claude.com/claude-code) (Claude Opus 4.8)
1 parent d9fae25 commit 2266020

5 files changed

Lines changed: 160 additions & 1 deletion

File tree

‎bindings/pyroot/pythonizations/test/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ ROOT_ADD_PYUNITTEST(pyroot_pyz_ttree_branch ttree_branch.py PYTHON_DEPS numpy)
5959

6060
# TColor-related pythonizations
6161
ROOT_ADD_PYUNITTEST(pyroot_pyz_tcolor tcolor.py)
62+
ROOT_ADD_PYUNITTEST(regression_20018 regression_20018.py)
6263

6364
# TH1 and subclasses pythonizations
6465
ROOT_ADD_PYUNITTEST(pyroot_pyz_th1 th1.py)

‎bindings/pyroot/pythonizations/test/regression_20018.py‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import unittest
2+
3+
import ROOT
4+
5+
6+
class Regression20018(unittest.TestCase):
7+
"""
8+
Regression test for https://github.com/root-project/root/issues/20018
9+
10+
Calling ``TColor.DefinedColors(1)`` puts the global color bookkeeping into
11+
the "always store colors" mode (``gLastDefinedColors = -1``). When a canvas
12+
is afterwards serialized to JSON for the web display - which is exactly what
13+
JupyROOT does to show a canvas inline in a notebook - ``TCanvas::Streamer``
14+
used to unconditionally call ``fPrimitives->Add(...)`` to attach the list of
15+
colors. During the web-canvas snapshot the pad primitives are temporarily
16+
detached (``fPrimitives`` is set to ``nullptr``, see
17+
``TWebCanvas::CreatePadSnapshot``), so this dereferenced a null pointer and
18+
crashed with a segmentation violation.
19+
"""
20+
21+
def test_definedcolors_web_canvas_json(self):
22+
# TWebCanvas (and thus CreateCanvasJSON) is only available when ROOT is
23+
# built with the web display (root7/webgui). Without it, JupyROOT uses a
24+
# different code path that is not affected by this issue.
25+
if not hasattr(ROOT, "TWebCanvas") or not hasattr(ROOT.TWebCanvas, "CreateCanvasJSON"):
26+
self.skipTest("ROOT was built without the web display (webgui)")
27+
28+
ROOT.gROOT.SetBatch(True)
29+
30+
# This is what triggers the bug: force the "always store colors" mode.
31+
ROOT.TColor.DefinedColors(1)
32+
33+
c = ROOT.TCanvas("c_regression_20018", "Basic ROOT Plot", 800, 600)
34+
h = ROOT.TH1F("h_regression_20018", "Example Histogram;X axis;Entries", 100, 0, 10)
35+
h.FillRandom("gaus", 1000)
36+
h.Draw()
37+
c.Update()
38+
39+
# Used to segfault in TCanvas::Streamer due to a null fPrimitives.
40+
json = ROOT.TWebCanvas.CreateCanvasJSON(c, 23, True)
41+
self.assertGreater(len(json.Data()), 0)
42+
43+
44+
if __name__ == "__main__":
45+
unittest.main()

‎graf2d/gpad/src/TCanvas.cxx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2353,7 +2353,13 @@ void TCanvas::Streamer(TBuffer &b)
23532353
//in the buffer, do not add the list of colors to the list of primitives.
23542354
TObjArray *colors = nullptr;
23552355
TObjArray *CurrentColorPalette = nullptr;
2356-
if (TColor::DefinedColors()) {
2356+
// fPrimitives can be temporarily null while streaming a web-canvas
2357+
// snapshot (see TWebCanvas::CreatePadSnapshot). In that case colors and
2358+
// palette are delivered separately, so the list of colors must not be
2359+
// added here. The guard also avoids a null dereference that crashes when
2360+
// colors storage has been forced on via TColor::DefinedColors(1)
2361+
// (see https://github.com/root-project/root/issues/20018).
2362+
if (fPrimitives && TColor::DefinedColors()) {
23572363
if (!b.CheckObject(gROOT->GetListOfColors(),TObjArray::Class())) {
23582364
colors = (TObjArray*)gROOT->GetListOfColors();
23592365
fPrimitives->Add(colors);

‎roottest/python/JupyROOT/CMakeLists.txt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,18 @@ if(imt)
4949
PYTHON_DEPS jupyter)
5050
endif()
5151

52+
# Regression test for https://github.com/root-project/root/issues/20018 :
53+
# TColor.DefinedColors(1) followed by drawing a canvas inline used to segfault.
54+
# This notebook draws a canvas, so its output (unique div ids, random data) is
55+
# not reproducible - just check that it runs without error ("OFF", no compare).
56+
set(TCOLOR_NB tcolor_definedcolors.ipynb)
57+
get_filename_component(NOTEBOOKBASE ${TCOLOR_NB} NAME_WE)
58+
ROOTTEST_ADD_TEST(${NOTEBOOKBASE}_notebook
59+
COPY_TO_BUILDDIR ${TCOLOR_NB}
60+
COMMAND ${Python3_EXECUTABLE} ${NBDIFFUTIL} ${TCOLOR_NB} "OFF"
61+
RUN_SERIAL
62+
PYTHON_DEPS jupyter)
63+
5264
endif()
5365

5466
endif()

‎roottest/python/JupyROOT/tcolor_definedcolors.ipynb‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
{
2+
"cells": [
3+
{
4+
"cell_type": "code",
5+
"execution_count": null,
6+
"id": "b8ef0e09",
7+
"metadata": {},
8+
"outputs": [],
9+
"source": [
10+
"import ROOT"
11+
]
12+
},
13+
{
14+
"cell_type": "code",
15+
"execution_count": null,
16+
"id": "ac871508",
17+
"metadata": {},
18+
"outputs": [],
19+
"source": [
20+
"ROOT.TColor.DefinedColors(1)"
21+
]
22+
},
23+
{
24+
"cell_type": "code",
25+
"execution_count": null,
26+
"id": "4800d2c7",
27+
"metadata": {},
28+
"outputs": [],
29+
"source": [
30+
"c = ROOT.TCanvas(\"c\", \"Basic ROOT Plot\", 800, 600)"
31+
]
32+
},
33+
{
34+
"cell_type": "code",
35+
"execution_count": null,
36+
"id": "f3900d8a",
37+
"metadata": {},
38+
"outputs": [],
39+
"source": [
40+
"h = ROOT.TH1F(\"h\", \"Example Histogram;X axis;Entries\", 100, 0, 10)"
41+
]
42+
},
43+
{
44+
"cell_type": "code",
45+
"execution_count": null,
46+
"id": "d83cc9da",
47+
"metadata": {},
48+
"outputs": [],
49+
"source": [
50+
"for _ in range(10000):\n",
51+
" h.Fill(ROOT.gRandom.Gaus(5, 1))"
52+
]
53+
},
54+
{
55+
"cell_type": "code",
56+
"execution_count": null,
57+
"id": "1df7d032",
58+
"metadata": {},
59+
"outputs": [],
60+
"source": [
61+
"h.Draw()"
62+
]
63+
},
64+
{
65+
"cell_type": "code",
66+
"execution_count": null,
67+
"id": "3743fddc",
68+
"metadata": {},
69+
"outputs": [],
70+
"source": [
71+
"c.Draw()"
72+
]
73+
}
74+
],
75+
"metadata": {
76+
"kernelspec": {
77+
"display_name": "Python 3",
78+
"language": "python",
79+
"name": "python3"
80+
},
81+
"language_info": {
82+
"codemirror_mode": {
83+
"name": "ipython",
84+
"version": 3
85+
},
86+
"file_extension": ".py",
87+
"mimetype": "text/x-python",
88+
"name": "python",
89+
"nbconvert_exporter": "python",
90+
"pygments_lexer": "ipython3"
91+
}
92+
},
93+
"nbformat": 4,
94+
"nbformat_minor": 5
95+
}

0 commit comments

Comments
 (0)