'conflicting chdir during another chdir block' calling vite_asset_path

[JasonBarnabe]

• [ ] I have tried upgrading by running bundle update vite_ruby.
• [x] I have read the troubleshooting section before opening an issue.

Description 📖

I have a page with a number of iframes loading PDF previews. When loading, some of them fail with:

RuntimeError: conflicting chdir during another chdir block (RuntimeError)
  from vite_ruby (3.3.4) lib/vite_ruby/config.rb:84:in `chdir'
  from vite_ruby (3.3.4) lib/vite_ruby/config.rb:84:in `within_root'
  from vite_ruby (3.3.4) lib/vite_ruby/builder.rb:57:in `watched_files_digest'
  from vite_ruby (3.3.4) lib/vite_ruby/builder.rb:32:in `last_build_metadata'
  from vite_ruby (3.3.4) lib/vite_ruby/manifest.rb:217:in `missing_entry_error'
  from vite_ruby (3.3.4) lib/vite_ruby/manifest.rb:79:in `lookup!'
  from vite_ruby (3.3.4) lib/vite_ruby/manifest.rb:22:in `path_for'
  from vite_rails (3.0.17) lib/vite_rails/tag_helpers.rb:26:in `vite_asset_path'

When loading them individually, they work. This seems like a thread safety issue, as chdir is not thread-safe.

Vite Ruby Info

bin/vite present?: true
vite_ruby: 3.3.4
vite_rails: 3.0.17
rails: 7.0.8
node: v20.11.0
npm: 10.2.4
yarn: 1.22.5
pnpm: 7.14.2
ruby: ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-linux]

[kennethkalmer]

We've just stumbled onto the same issue testing our migration to vite_rails. Still have some investigation to do, but my initial exploration points to a small difference in behaviour between the two branches in ViteRuby::Manifest.lookup!.

One of the branches is aware of whether we should build the assets or not, while the error branch isn't.

  def lookup(name, **options)
    @build_mutex.synchronize { builder.build || (return nil) } if should_build?


    find_manifest_entry resolve_entry_name(name, **options)
  end

Should missing_entry_error also be aware of the @build_mutex?

  def missing_entry_error(name, **options)
    raise ViteRuby::MissingEntrypointError.new(
      file_name: resolve_entry_name(name, **options),
      last_build: builder.last_build_metadata,
      manifest: @manifest,
      config: config,
    )
  end

[kennethkalmer]

To add some more context from our use case. We have a convenience pattern in our application where we dynamically try and match entrypoints (or "packs" in Webpacker parlance) to the current controller and/or action. We have a helper method that basically looks like this:

  def vite_controller_packs(pack_type = :javascript)
    controller_pack = "entrypoints/controllers/#{params[:controller]}/#{params[:controller]}"
    controller_action_pack = "entrypoints/controllers/#{params[:controller]}/#{params[:action]}"

    [controller_pack, controller_action_pack]
      .select { |pack_name|
        begin
          ViteRuby.instance.manifest.path_for(pack_name, type: pack_type).present?
        rescue ViteRuby::MissingEntrypointError
        end
      }
  end

In our testing environment we're running Puma with multiple threads and so we can reliably generate this race condition of going down the missing_entry_error branch of ViteRuby::Manifest#lookup!.

Our remedy for the moment is prepend a new method to ViteRuby::Manifest which calls the protected lookup method and work with its return value, allowing us to skip the error branch and raising unnecessary exceptions altogether.

[ElMassimo]

Rewriting watched_files_digest to manually resolve config.watched_paths instead of using chdir seems reasonable.