Allow suspending fibers in destructors #11389

Activity

1. nicolas-grekas

   added

   Feature

   Status: Needs Triage

   on Jun 7, 2023
2. arnaud-lb commented on Jun 12, 2023

   

   arnaud-lb

   on Jun 12, 2023

   Member

   More actions

   Destructors/finalizers exist for the kind of use-cases that may require I/O (e.g. closing a network connection or flushing a buffer). With an I/O framework that uses Fibers, this may necessitate Fiber switching, so providing support for this would be beneficial.

   One solution would be to execute the destructors after collection. This seems to be common practice because this solves a number of issues.

   However, this raises the following question: Does the Fiber in which the destructors are called matter? I'm not very familiar with Amp internals: does it matter if the caller may be any Fiber, including the scheduler Fiber (if there is one) or the main "Fiber"?

   An other issue, similar to the one raised by @trowski in nicolas-grekas/symfony#43 (comment), is that if a destructor suspends and the Fiber is never resumed, the remaining destructors may never be called. A solution to this might be to execute the remaining destructors after switching.
3. arnaud-lb

   added

   Category: Fibers

   Category: GC

   on Jun 12, 2023
4. mvorisek commented on Jun 23, 2023

   

   mvorisek

   on Jun 23, 2023

   Last edited by mvorisek

   Contributor

   More actions

   if a destructor suspends and the Fiber is never resumed, the remaining destructors may never be called. A solution to this might be to execute the remaining destructors after switching.

   This is already the current behaviour - https://3v4l.org/6AvfW - finally blocks and destructors are executed in the same fiber even if suspended.
5. arnaud-lb commented on Jun 24, 2023

   

   arnaud-lb

   on Jun 24, 2023

   Last edited by arnaud-lb

   Member

   More actions

   Yes, currently the destructors are called in the Fiber that happened to be active when the GC was triggered. What I meant is that if the GC queues 2 destructors, and one suspends, then the other one may never be executed. So I was suggesting that after suspend, we execute the remaining destructors in the new active Fiber.

   class A {
       public function __destruct() { Fiber::suspend(); }
   }
   class B {
       public function __destruct() { }
   }
   
   $x = new stdClass;
   $x->x = $x;
   $x->a = new A;
   $x->b = new B;
   $x = null;
   gc_collect_cycles(); // Would queue $a->__destruct() and $b->__destruct()
   

   In the example above, when $a->__destruct() suspends, $b->__destruct() would be executed in the fiber we switched to after suspending.

   👍React with 👍1Reacted by Michael Voříšek
6. arnaud-lb commented on Jun 24, 2023

   

   arnaud-lb

   on Jun 24, 2023

   Last edited by arnaud-lb

   Member

   More actions

   This made me realize that fiber switching is not only disallowed during GC, but also when destroying objects outside of GC.

   @trowski @kooldev do you remember why you had to disable switching in fdc22744a89 ?
7. trowski commented on Jul 9, 2023

   

   trowski

   on Jul 9, 2023

   Member

   More actions

   does it matter if the caller may be any Fiber, including the scheduler Fiber (if there is one) or the main "Fiber"?

   Generally it does not matter which fiber calls the destructor. We wouldn't want a suspending destructor to be invoked within the event loop (scheduler) fiber, but we wrote the event loop to execute callbacks and destroy the reference to that callback within another fiber separate from the event loop fiber.

   @trowski @kooldev do you remember why you had to disable switching in fdc22744a89 ?

   We disabled switching fibers in all contexts for consistency. It would be surprising to be able to switch fibers in a destructor, unless the destructor was being run as part of cyclic garbage collection.

   ---

   To make fiber switching in destructors safe, it appears we do not need to make the garbage collector itself safe for re-entry if we collect objects to be destructed, then execute destructors after collection. That way no user code can be executed during cyclic collection. We then only need to ensure that a destructor suspending during execution does not block the destructor of other objects collected during garbage collection.

   To accomplish this, I think we can use a strategy similar to that which we used in the Revolt event loop to execute loop callbacks, which also may suspend during execution and should not block other loop callbacks from being immediately executed. After garbage collection collects the objects to be destroyed, we switch fibers before executing the queued destructors. If the fiber is not suspended within a destructor, we can execute the next destructor on the same fiber. If that fiber does suspend in the destructor, a new fiber is allocated to execute the next destructor, replacing the fiber which was suspended.

   A fiber suspended during destructor execution will live on in use space, but once resumed, returning from the destructor, the fiber will be destroyed because no further references will exist in either user space or internally.

   This strategy would require a global to store the fiber for executing destructors and at least one fiber switch before and after executing destructors. Fiber switches are very fast and would only occur after the GC is run, not on every destructor call. The fiber for executing destructors would only need to be created when cycles are collected by the GC.

   @arnaud-lb I believe you might have been alluding to this strategy in your post above. Is this something you'd have time to try implementing?
8. arnaud-lb commented on Jul 16, 2023

   

   arnaud-lb

   on Jul 16, 2023

   Member

   More actions

   @arnaud-lb I believe you might have been alluding to this strategy in your post above. Is this something you'd have time to try implementing?

   I was indeed alluding to a similar strategy. You have gone a bit further though, by running the destructors in a separate fiber so that a suspending destructor will not disrupt the application's fibers.

   This should probably target 8.4. There are a few things I want to do before working on this, but I should be able to implement it before 8.4 (unless you or someone else want to, of course).
9. arnaud-lb

   closed this as completedin 3c56af9on Jul 2, 2024
10. Girgias

    added a commit that references this issue on Jul 2, 2024

    Allow fiber switching during destructor execution

    ...

    This issue will close once commit 3c56af9 is merged into the 'master' branch.

    3c56af9
11. fabpot

    added a commit that references this issue on Aug 19, 2024

    feature #54179 [HttpClient] Add support for amphp/http-client v5 (nic…

    ...

    a44311e