Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: infinite loop when 2 factories refer to each other in their default() methods #724

Closed
simondaigre opened this issue Nov 22, 2024 · 9 comments · Fixed by #742
Closed
Labels
bug Something isn't working

Comments

@simondaigre
Copy link
Contributor

simondaigre commented Nov 22, 2024

Since 2.x on one of my projects, I have a segfault when I try to create a complex PersistentProxyObject with a lot of dependencies.
On 1.x no issues at all.

I dig a lot to find what's wrong but I have no clue.
Reproducer is here : https://github.com/simondaigre/foundry-repro/tree/repro-segfault

@nikophil
Copy link
Member

Ok, thanks for the reproducer, I'll check this when I got some time

@kbond
Copy link
Member

kbond commented Nov 22, 2024

@simondaigre, 404 when accessing your reproducer. Maybe not public?

@simondaigre
Copy link
Contributor Author

simondaigre commented Nov 22, 2024

@simondaigre, 404 when accessing your reproducer. Maybe not public?

You're right, I just made it public.

@nikophil
Copy link
Member

nikophil commented Dec 12, 2024

hi @simondaigre

the problem is not a "complex schema" or a lot of dependencies.
You have an infinite loop:

final class AgencyFactory extends PersistentProxyObjectFactory
{
    protected function defaults(): array
    {
        return [
            'agencyImages' => AgencyImageFactory::new()->many(self::faker()->randomDigitNotNull()),
        ];
    }
}

final class AgencyImageFactory extends PersistentProxyObjectFactory
{
    protected function defaults(): array
    {
        return [
            'agency' => AgencyFactory::new(),
        ];
    }
}

when I do dump(static::class), It prints infinitely:

^ "App\Factory\AgencyFactory"
^ "App\Factory\AgencyImageFactory"
^ "App\Factory\AgencyFactory"
^ "App\Factory\AgencyImageFactory"
^ "App\Factory\AgencyFactory"
^ "App\Factory\AgencyImageFactory"
...

As stated in the docs, you should avoid adding ...ToMany relationships in the defaults() method:
Capture d’écran du 2024-12-12 10-39-57

That being said, I think we should implement some kind of circular dependency guard.

I'm also wondering how Foundry 1 did handle this 🤔

@nikophil nikophil changed the title [2.x] Segfault on complex schema bug: infinite loop when 2 factories refer to each other in their default() methods Dec 12, 2024
@nikophil nikophil added the bug Something isn't working label Dec 12, 2024
@simondaigre
Copy link
Contributor Author

simondaigre commented Dec 12, 2024

@nikophil thanks for looking at this !

I can handle this but it could be simpler if Foundry handle this. Sometime I just need to instanciate some AgencyImage, sometimes I need Agency instance with AgencyImage.

I tried to remove AgencyImage and upgrade to Foundry 2 and now I have a lot of A new entity was found through the relationship xxx that was not configured to cascade persist operations. on other tests. I'll check this new issue later.

@nikophil
Copy link
Member

I can handle this but it could be simpler if Foundry handle this. Sometime I just need to instanciate some AgencyImage, sometimes I need Agency instance with AgencyImage.

I'm not sure we'll actively support the fact to add OneToMany in the default() method. I'll have to think about it, but it used to lead to surprising results in Foundry 1 (unfortunately I cannot find the issue where this was stated).

I tried to remove AgencyImage and upgrade to Foundry 2 and now I have a lot of A new entity was found through the relationship xxx that was not configured to cascade persist operations. on other tests. I'll check this new issue later.

you can either add cascade: ['persist'] to your relationships. Or wait for #742 to be released. I think it will fix your problems

@nikophil
Copy link
Member

nikophil commented Dec 12, 2024

fun fact: #742 seems to also fix the infinite loop problem, and I absolutely don't know why 😅

by the way, I tried to reproduce the problem in foundry's test suite, and I did not manage to do it, even without the fix 🤷

@simondaigre
Copy link
Contributor Author

I just checked and I confirm #742 fixes all my issues. All my tests are OK, like Foundry 1 🎉

@nikophil
Copy link
Member

this will be released soon 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

3 participants