Skip to content

OCCTSectionBuilder's built flag never resets to false on a failed rebuild (same class as #910) #916

Description

@gsdali

OCCTSwift version

Current main (found during PR #912's review, unreleased).

Platform

macOS (arm64), likely all platforms — bridge code, not platform-specific.

Description

OCCTSectionBuilder (Sources/OCCTBridge/src/OCCTBridge_Modeling.mm, backs SectionBuilder) has
the identical staleness defect PR #912 just fixed for OCCTThruSections, in a sibling reused
builder in the same file:

struct OCCTSectionBuilder {
    BRepAlgoAPI_Section section;
    bool built;
    OCCTSectionBuilder() : section(), built(false) {}
    OCCTSectionBuilder(const TopoDS_Shape& s1, const TopoDS_Shape& s2) : section(s1, s2, false), built(false) {}
};

OCCTShapeRef OCCTSectionBuilderBuild(OCCTSectionBuilderRef builder) {
    if (!builder) return nullptr;
    try {
        builder->section.Build();
        if (!builder->section.IsDone()) return nullptr;
        builder->built = true;
        return new OCCTShape{builder->section.Shape()};
    } catch (...) { return nullptr; }
}

OCCTShapeRef OCCTSectionBuilderAncestorFaceOn1(OCCTSectionBuilderRef builder, OCCTShapeRef edge) {
    if (!builder || !edge || !builder->built) return nullptr;
    ...
}

built is only ever set to true, on a successful build. On a failure — !IsDone() — the
function returns nullptr without ever resetting built back to false. So on a SectionBuilder
reused via init1(...)/init2(...) + build() (matching the reuse pattern PR #912's own
regression tests exercise for ThruSectionsBuilder), a builder that already built successfully
once keeps built == true through a later failed rebuild — ancestorFaceOn1(edge:) and
ancestorFaceOn2(edge:) would then skip their guard and read state from the failed/incomplete
rebuild's BRepAlgoAPI_Section instance instead of correctly returning nil.

Steps to reproduce

Not yet reduced to a minimal repro that reliably makes BRepAlgoAPI_Section::Build() itself fail
(IsDone() == false) — a quick attempt using non-intersecting/far-apart shapes did not trigger a
failure (the section is just empty, still IsDone() == true). Whoever picks this up needs to find
an actual failure trigger for BRepAlgoAPI_Section::Build() first (an invalid plane/surface
argument via init1(plane:)/init2(surface:) is the likely candidate, unexplored) before this can
be proven live the way PR #912's own regression tests were.

Notes

Found during PR #912's review (the #910 fix) while confirming that built is an established
naming convention in this bridge (it is — this struct predates #912's own use of the same name for
OCCTThruSections). Filed separately per this project's scope-boundary policy rather than folded
into #912, and because it needs its own live repro before a fix can be validated the same way #910
was.

Candidate fix, mirroring #912's own pattern: builder->built = false; on the !IsDone() path (and
on the catch (...) path) in OCCTSectionBuilderBuild, before returning nullptr.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    type:bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions