[Feature Request] Ensure fibers and workflow instances are properly GC'd on workflow eviction

Open
#334 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
25/100
Issue type
Feature
Clarity
Needs clarification
Activity status
Stale
Tech stack
ruby
Domain
backend, distributed-systems

Research direction

Start by reviewing the first comment and the skipped test_confirm_garbage_collect test, including its existing utilities and designs. Establish whether workflow eviction can leave fibers or workflow instances uncollected, then define a testable eviction scenario whose completion confirms they are garbage collected without unintended side effects.

Written by the indexing model from the issue text.

Description

enhancement
Describe the solution you'd like

It was originally thought that if nothing in user code was referencing a suspended fiber anymore, it would get garbage collected (this is how tasks work in .NET). However, threads actually keep a strong reference to suspended fibers and we reuse threads. On workflow eviction, any number of fibers may be suspended, including the primary one.

The only way to remove a strong reference to a fiber on a thread is to complete the fiber, and the only way to complete the fiber is resume until complete (potentially raising an exception inside it to force it to resume). So we should go over known fibers and raise a non-standard-error exception inside them. This needs to take an approach similar to https://github.com/temporalio/sdk-python/pull/499 where we ignore any side-effects that could be caused by raising (e.g. don't make an activity command if the user did it inside ensure). Make sure there is a test that tries to make uncollected fibers in any way it can and confirm. The test_confirm_garbage_collect test (that we had to skip pending this issue) has some utilities/designs here.

EDIT: These statements about threads holding strong references to fibers are no longer deemed accurate, see first comment.

Dominant language
Ruby
Stars
204
Forks
42
Avg merge
1d 30m
Merged PRs (30d)
28

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from temporalio/sdk-ruby

All issues in temporalio/sdk-ruby

Similar issues

More Ruby issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.