BUG: fix an infinite loop in itk::VoronoiDiagram2DGenerator by ILoveGoulash · Pull Request #5400 · InsightSoftwareConsortium/ITK

ILoveGoulash · 2025-06-16T13:43:45Z

The loop exit condition was present in the code, but never wired and removed via cd97879

PR Checklist

No API changes were made (or the changes have been approved)
No major design changes were made (or the changes have been approved)
Added test (or behavior not changed)
Updated API documentation (or API not changed)
Added license to new files (if any)
Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for
further development details if necessary.

github-actions

Thank you for contributing a pull request! 🙏

Welcome to the ITK community! 🤗👋☀️

We are glad you are here and appreciate your contribution. Please keep in mind our community participation guidelines. 📜
More support and guidance on the contribution process can be found in our contributing guide. 📖

This is an automatic message. Allow for time for the ITK community to be able to read the pull request and comment
on it.

ILoveGoulash · 2025-06-16T13:46:34Z

Don't know if you want a regression test, and if your framework can handle "infinite looping" type failures.

dzenanz · 2025-06-16T13:48:37Z

A regression test is advantageous. I don't think we have an explicit support for infinite looping, but such a test would fail via timeout.

ILoveGoulash · 2025-06-16T14:31:36Z

Added the original case by doing "monkey see monkey do", but it looks like I can't build locally due to IPFS + work firewall or something.

dzenanz

OK, I can reproduce locally an infinite loop with this test.

dzenanz · 2025-06-16T14:49:28Z

@bradking Is ghostflow acting up? Commit 62457de has a very short commit message.

ILoveGoulash · 2025-06-16T14:50:44Z

Nah, the full "http://..." link is probably what triggers it, GitHub prettifies it on display.

Modules/Segmentation/Voronoi/include/itkVoronoiDiagram2DGenerator.hxx

hjmjohnson · 2025-08-13T15:01:27Z

@ILoveGoulash Thank you for your contribution. There are just a few final actions that need to be finalized so that we can incorporate this into the main tree.

ILoveGoulash · 2025-08-14T07:24:44Z

Should be okay now.

hjmjohnson · 2025-08-14T12:58:41Z

@ILoveGoulash I rebased and removed the merge conflict, then updated the commit message for the regression test to be more descriptive.

ILoveGoulash · 2025-08-14T13:01:43Z

Thanks!

ILoveGoulash · 2025-11-17T15:45:59Z

Ping.

The loop exit condition was present in the code, but never wired and removed via InsightSoftwareConsortium@cd97879 Fixes InsightSoftwareConsortium#4386

Tests and resolves InsightSoftwareConsortium#4386 where specific values caused deadlocks.

N-Dekker · 2026-01-28T16:52:23Z

Modules/Segmentation/Voronoi/test/itkVoronoiDiagram2DInfiniteLoopTest.cxx

+itkVoronoiDiagram2DInfiniteLoopTest(int argc, char * argv[])
+{
+  if (argc != 1)
+  {
+    std::cerr << "Takes no parameters, but " << argc << "parameters given" << std::endl;


When the test has no parameters, preferably make it a GoogleTest. GoogleTest unit tests are typically more "to the point", avoiding unnecessary verbosity.

N-Dekker · 2026-01-28T17:01:20Z

Modules/Segmentation/Voronoi/include/itkVoronoiDiagram2DGenerator.hxx

    EdgeInfo back = curr;
-    while (!(rawEdges[i].empty()))
+    auto     maxStop = rawEdges[i].size();
+    while (!(rawEdges[i].empty()) && (maxStop != 0))


Do I understand correctly that the check !(rawEdges[i].empty()) is redundant now? I mean, could it just say:

while ( maxStop > 0 )

If so, I would prefer such a simplification.

Couldn't simply be a for loop?

for ( auto maxStop = rawEdges[i].size(); maxStop != 0; --maxStop )

Maybe then rename maxStop to j:

for ( auto j = rawEdges[i].size(); j != 0; --j )

@N-Dekker I've looked into this some more. The problem is more difficult than the requested change you proposed. I think the issue is that we are running into a corner case of the algorithm that is not handled.

This PR adds a test that induces an infinite loop. The initially requested change breaks the infinite loop but changes the algorithm's behavior. (other test starts failing).

I haven't figured out what needs to be done, but it likely requires careful consideration of the implemented algorithm and more careful identification of corner cases.

The crux of the problem is removing items from the list for processing, then adding them back to the list if none of the described cases are met (i.e. an infinite loop occurs).

Add stall detection to the edge-assembly loop that builds Voronoi cell boundaries. The loop pops edges from a deque and tries to attach them to a growing chain; unattachable edges are pushed back for retry. With certain degenerate seed configurations (ITK issue InsightSoftwareConsortium#4386), some edges can never attach, causing the loop to run forever. The fix tracks a stall counter that is reset whenever an edge successfully attaches (since new attachments change the chain endpoints and may make previously unattachable edges attachable). When a full pass through the deque makes no progress, the loop terminates. This improves on the approach in PR InsightSoftwareConsortium#5400, which used a simple countdown that could terminate too early — causing itkVoronoiPartitioningImageFilterTest2 to fail because edges that would have been attachable in a subsequent pass were prematurely dropped. A regression test using the seed configuration from issue InsightSoftwareConsortium#4386 is included.

Fix an infinite loop in ConstructDiagram() that occurs with certain degenerate seed configurations (ITK issue InsightSoftwareConsortium#4386, supersedes PR InsightSoftwareConsortium#5400). Root cause: Fortune's algorithm produces near-zero-length edges when boundary intersection points coincide within floating-point tolerance but receive different vertex IDs. These edges cannot attach to the growing cell boundary chain because: 1. Their vertex IDs don't match any chain endpoint. 2. The chain may already be closed (front == back). 3. Boundary-bridging logic doesn't apply when chain endpoints are interior (not on the domain boundary). The fix has three parts: 1. Explicit degenerate edge detection: before attempting attachment, each edge is checked with the existing differentPoint() tolerance (DIFF_TOLERENCE = 0.001). Edges whose endpoints are geometrically identical are discarded immediately — they carry no geometric information and are floating-point artifacts. 2. Stall detection: a counter tracks assembly progress and resets whenever an edge attaches (since new attachments change the chain endpoints and may make previously unattachable edges attachable). When a full pass through the deque makes no progress, the loop terminates. This improves on PR InsightSoftwareConsortium#5400's simple countdown which terminated too early, breaking itkVoronoiPartitioningImageFilterTest2. 3. Exception on unexpected residuals: after assembly, if any non-degenerate edges remain, itkExceptionMacro fires to ensure unexpected geometric configurations are reported rather than silently producing incomplete diagrams. A regression test using the seed configuration from issue InsightSoftwareConsortium#4386 verifies termination and validates all cell boundaries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

github-actions bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances area:Segmentation Issues affecting the Segmentation module labels Jun 16, 2025

github-actions bot reviewed Jun 16, 2025

View reviewed changes

dzenanz requested review from hjmjohnson and seanm June 16, 2025 13:45

github-actions bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct labels Jun 16, 2025

ILoveGoulash force-pushed the fix-voronoi-diagram-2d-generator-inf-loop branch 2 times, most recently from f5ff22b to 62457de Compare June 16, 2025 14:39

dzenanz reviewed Jun 16, 2025

View reviewed changes

ILoveGoulash force-pushed the fix-voronoi-diagram-2d-generator-inf-loop branch from 62457de to 288b65e Compare June 16, 2025 14:51

hjmjohnson reviewed Jun 17, 2025

View reviewed changes

Modules/Segmentation/Voronoi/include/itkVoronoiDiagram2DGenerator.hxx Outdated Show resolved Hide resolved

ILoveGoulash force-pushed the fix-voronoi-diagram-2d-generator-inf-loop branch from 288b65e to 380c33d Compare August 14, 2025 07:24

hjmjohnson force-pushed the fix-voronoi-diagram-2d-generator-inf-loop branch 2 times, most recently from 8f868e4 to c6ccef6 Compare August 14, 2025 12:57

dzenanz approved these changes Aug 14, 2025

View reviewed changes

hjmjohnson force-pushed the fix-voronoi-diagram-2d-generator-inf-loop branch from c6ccef6 to cec88e2 Compare August 14, 2025 16:05

dzenanz approved these changes Aug 14, 2025

View reviewed changes

dzenanz assigned hjmjohnson Nov 17, 2025

bradking force-pushed the main branch from 150da41 to 77e3bef Compare November 17, 2025 21:54

ILoveGoulash added 2 commits January 28, 2026 09:31

BUG: fix an infinite loop in itk::VoronoiDiagram2DGenerator

51ad731

The loop exit condition was present in the code, but never wired and removed via InsightSoftwareConsortium@cd97879 Fixes InsightSoftwareConsortium#4386

ENH: Add test of infinite loop in VoronoiDiagram2DGenerator

ddccb03

Tests and resolves InsightSoftwareConsortium#4386 where specific values caused deadlocks.

hjmjohnson force-pushed the fix-voronoi-diagram-2d-generator-inf-loop branch from 4aa2b26 to ddccb03 Compare January 28, 2026 15:33

N-Dekker reviewed Jan 28, 2026

View reviewed changes

hjmjohnson mentioned this pull request Apr 7, 2026

BUG: Fix infinite loop in VoronoiDiagram2DGenerator #6017

Merged

Uh oh!

Conversation

ILoveGoulash commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

ILoveGoulash commented Jun 16, 2025

Uh oh!

dzenanz commented Jun 16, 2025

Uh oh!

ILoveGoulash commented Jun 16, 2025

Uh oh!

dzenanz left a comment

Choose a reason for hiding this comment

Uh oh!

dzenanz commented Jun 16, 2025

Uh oh!

ILoveGoulash commented Jun 16, 2025

Uh oh!

Uh oh!

hjmjohnson commented Aug 13, 2025

Uh oh!

ILoveGoulash commented Aug 14, 2025

Uh oh!

hjmjohnson commented Aug 14, 2025

Uh oh!

ILoveGoulash commented Aug 14, 2025

Uh oh!

ILoveGoulash commented Nov 17, 2025

Uh oh!

N-Dekker Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

N-Dekker Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

N-Dekker Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

hjmjohnson Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ILoveGoulash commented Jun 16, 2025 •

edited

Loading

hjmjohnson Feb 17, 2026 •

edited

Loading