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

Do not run tasks when their prerequisites fail #142

Merged
merged 1 commit into from
Jun 22, 2016

Conversation

ptarjan
Copy link
Contributor

@ptarjan ptarjan commented Jun 22, 2016

If tasks A and B both depend on C, they race to grab the invocation
lock on C. If A wins, it sets @already_invoked and executes C while
holding the lock. When C completes, A releases the lock, B acquires it
and immediately returns since C is @already_invoked. Unfortunately,
this does not distinguish failed execution of C. Instead, if executing
C raises an exception for A, we want to raise the same exception in B
so it is aborted.

@drbrain
Copy link
Member

drbrain commented Jun 22, 2016

Can you add a test for this as well?

I think something like this will adequately simulate cross-thread exceptions (but I didn't try it):

  def test_cross_thread_prerequisite_failures
    failed = false

    task :fail_once do
      fail_now = !failed
      failed = true
      raise 'failing once' if fail_now
    end

    task a: :fail_once
    task b: :fail_once

    assert_raises RuntimeError do
      Rake::Task[:a].invoke
    end

    assert_raises RuntimeError do
      Rake::Task[:b].invoke
    end
  end

If tasks A and B both depend on C, they race to grab the invocation
lock on C. If A wins, it sets @already_invoked and executes C while
holding the lock. When C completes, A releases the lock, B acquires it
and immediately returns since C is @already_invoked. Unfortunately,
this does not distinguish failed execution of C. Instead, if executing
C raises an exception for A, we want to raise the same exception in B
so it is aborted.
@ptarjan
Copy link
Contributor Author

ptarjan commented Jun 22, 2016

@drbrain added. Thanks for the starting point. I had to make one of them a multitask to run my code

@drbrain drbrain merged commit d07fd9f into ruby:master Jun 22, 2016
@ptarjan ptarjan deleted the ptarjan-MultiTask branch June 22, 2016 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants