Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

Fix intermittent error ("can't add a new key into hash during iteration") in the theme push command #2112

Merged
merged 2 commits into from
Mar 7, 2022

Conversation

karreiro
Copy link
Contributor

@karreiro karreiro commented Mar 2, 2022

WHY are these changes introduced?

The error "can't add a new key into hash during iteration" appears intermittently when users run shopify theme push.

WHAT is this pull request doing?

The error mentioned in the #1705 issue happens at the update_checksums method (in the ShopifyCLI::Theme::Syncer class):

def update_checksums(api_response)
  api_response.values.flatten.each do |asset|
    if asset["key"]
      @checksums[asset["key"]] = asset["checksum"] # <-------------------------- It's raised here
    end
  end

  # [...]
  @checksums.reject! { |key, _| @checksums.key?("#{key}.liquid") } # <---------- Notice
end

Here's a simpler reproducer of the same error (with multiple threads modifying with the same Hash):

hash = {}

modify_hash = -> do
  hash[rand(1..99).to_s] = rand(1..99).to_s
  hash.reject! { |k, _v| k == rand(1..99).to_s }
end

t1 = Thread.new do
  loop do
    sleep(0.00005 * 1)
    modify_hash.call
  end
end

t2 = Thread.new do
  loop do
    sleep(0.00005 * 2)
    modify_hash.call
  end
end

t3 = Thread.new do
  loop do
    sleep(0.00005 * 3)
    modify_hash.call
  end
end

t1.join
t2.join
t3.join

This PR adds a semaphore to coordinate changes in the @checksums object (shared across multiple threads).

How to test your changes?

  • Run shopify-dev theme push -u -t <name> many times with different names in the -t flag
  • Notice the intermittent error no longer happens

Post-release steps

None.

Update checklist

  • I've added a CHANGELOG entry for this PR (if the change is public-facing)
  • I've considered possible cross-platform impacts (Mac, Linux, Windows).
  • I've left the version number as is (we'll handle incrementing this when releasing).
  • I've included any post-release steps in the section above.

Copy link
Contributor

@macournoyer macournoyer left a comment

Choose a reason for hiding this comment

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

Amazing! Nice catch 👏

@karreiro karreiro requested a review from a team March 7, 2022 16:07
@karreiro karreiro merged commit 78cd3dd into main Mar 7, 2022
@karreiro karreiro deleted the fix-1705 branch March 7, 2022 16:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Intermittent theme push error: "can't add a new key into hash during iteration"
2 participants