Upload: Keep the transaction around #1742
Conversation
With Mojo 7.88 Subprocesses are started not immediately anymore, but inside the IOLoop. To avoid transaction to vanish since it's weakened, explictly force it
|
Before: http://e122.suse.de/tests/22996 |
| return Mojo::IOLoop->subprocess( | ||
| sub { | ||
| $tx; |
There was a problem hiding this comment.
This is just to make sure that $tx ends up in the coderef context right? , loooks like something is missing
There was a problem hiding this comment.
There was a problem hiding this comment.
Yea, the controller object (Mojolicious::Controller) only keeps a weaked reference to the transaction ($c->tx). So if you want to use code depending on the transaction (like $c->param(...)) in async closures, you need to keep a strong reference to the transaction somewhere.
There was a problem hiding this comment.
From a framework design point of view this is a very tricky problem. If $c->tx wasn't a weakened reference you wouldn't have this problem here, but pretty much every Mojolicious application using async features would be required to use Scalar::Util::weaken all over the place or leak tons of memory with every request. So, i think this is actually the lesser evil. 😩
There was a problem hiding this comment.
@kraih Yeah, I guess so... having memory leaks all over the place sounds like a fun thing to debug :)
There was a problem hiding this comment.
Makes sense.
The failing fullstack test is unrelated (https://progress.opensuse.org/issues/39338).
Not so sure about the other failures but likely unrelated, too.
With Mojo 7.88 Subprocesses are started not immediately anymore, but
inside the IOLoop, a bit later. To avoid transaction to vanish since it's weakened,
explictly force it