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

Fix the bug with pipeline persisting in Zombie mode after self-terminating with terminate: :normal action #538

Merged
merged 5 commits into from
Mar 17, 2023

Conversation

varsill
Copy link
Contributor

@varsill varsill commented Mar 15, 2023

Some additional small changes (removing unused variables and aliases from tests) have also been performed since my credo has been failing.

@varsill varsill requested a review from mat-hek as a code owner March 15, 2023 09:30
@varsill varsill added the no-changelog This label has to be added if changes from the PR are not meant to be placed in the CHANGELOG.md label Mar 15, 2023
@varsill varsill requested a review from FelonEkonom March 15, 2023 11:24
test/membrane/core/pipeline_test.exs Outdated Show resolved Hide resolved
@@ -95,6 +95,8 @@ defmodule Membrane.Core.Pipeline.ActionHandler do

@impl CallbackHandler
def handle_action({:terminate, :normal}, _cb, _params, state) do
state = %{state | terminating?: true}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

callbacks above don't change state, so maybe move this line to LifecycleController.handle_terminate/1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the only usage of Parent.LifecycleController.handle_terminate/1 is here, so we can safely move it ;)

varsill and others added 3 commits March 16, 2023 12:03
Co-authored-by: Feliks Pobiedziński <38541925+FelonEkonom@users.noreply.github.com>
test/membrane/element_test.exs Outdated Show resolved Hide resolved
Co-authored-by: Mateusz Front <mateusz.front@swmansion.com>
@varsill varsill merged commit c325ae9 into master Mar 17, 2023
@varsill varsill deleted the bugfix/persistent_zombie_mode branch March 17, 2023 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog This label has to be added if changes from the PR are not meant to be placed in the CHANGELOG.md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants