Skip to content

Factory: #584's cycle guard is not reached from WithExisting* / AddExisting*, still stack-overflows in v0.50.0 #761

Description

@labsvisual

#584 fixed the recursion for FromExisting*. The guard is in v0.50.0, but WithExisting* and AddExisting* call the unexported fromExisting* directly, so they hit the recursion with a context that never got the visited set. The guard is skipped when the set isn't there, so on this path it does nothing.

Create hands back models with .R populated, so passing a parent you just created into a factory is enough to hit it. You get fatal error: stack overflow, which kills the whole test binary rather than failing one test.

Bob v0.50.0, bobgen-psql, Go 1.26.6, Postgres 18.1.

Repro

Two tables, one FK. employee has a to-one Org, org has a to-many Employees.

CREATE TABLE org (
    id   uuid PRIMARY KEY,
    name text NOT NULL
);

CREATE TABLE employee (
    id     uuid PRIMARY KEY,
    org_id uuid NOT NULL REFERENCES org (id),
    name   text NOT NULL
);

No DB, no Create, one hop:

func TestFactoryCycle(t *testing.T) {
    f := factory.New()

    org := &models.Org{}
    emp := &models.Employee{}
    emp.R.Org = org
    org.R.Employees = models.EmployeeSlice{emp}

    // fatal error: stack overflow
    _ = f.NewEmployee(factory.EmployeeMods.WithExistingOrg(org)).Build()
}

Fires every time.

Where

gen/templates/factory/bobfactory_main.bob.go.tpl:38 seeds the set inside FromExisting*. Line 53 only checks it if it's already there:

  if visited, ok := factoryVisitedCtx.Value(ctx); ok {

Both mods skip that wrapper:

  • gen/templates/factory/table/012_rel_to_one_mods.go.tpl:55
  • gen/templates/factory/table/013_rel_to_many_mods.go.tpl:59

Both call o.f.fromExisting{{$ftable.UpSingular}}(ctx, em) with whatever ctx they were handed, which for a mod is context.Background(). So ok is false and fromExistingEmployee -> WithExistingOrg -> fromExistingOrg -> AddExistingEmployees -> fromExistingEmployee runs until the stack goes.

Fix

Seed it in fromExisting* rather than in the wrapper. Replace line 53:

  visited, ok := factoryVisitedCtx.Value(ctx)
  if !ok {
    visited = make(map[uintptr]struct{})
    ctx = factoryVisitedCtx.WithValue(ctx, visited)
  }
  {
    ptr := uintptr(unsafe.Pointer(m))
    if _, seen := visited[ptr]; seen {
      return o
    }
    visited[ptr] = struct{}{}
  }

The ctx reassignment is the bit that matters -- the relationship loop below already passes ctx to Apply, so that's what carries the set down.

Lines 36-40 in FromExisting* can go after that. Same behaviour through that entry point, and one place creating the set instead of two, which is how the two paths drifted apart in the first place.

unsafe is already imported.

What I tested

Made the same change by hand in generated output (8 fromExisting* funcs):

Haven't run bob's tests against a modified template, so the template diff itself is untested -- only the logic it generates.

PR

Happy to raise it, with a regression test for the mods path. Also fine leaving the seeding in FromExisting* if you'd rather keep it explicit, no strong feelings either way.

Related

#584 is this bug through the other door. #510 touched the same mods -- on v0.50.0 WithoutParent no longer creates a phantom parent, checked that on the way past.

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

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions