Skip to content

Consider MT-safe refactor of ManifestUtil library #13645

@JanProvaznik

Description

@JanProvaznik

Summary

The ManifestUtil library (under src/Tasks/ManifestUtil/) contains several methods that internally use Directory.GetCurrentDirectory() and other process-global APIs. While MSBuild's built-in manifest tasks have been migrated to use TaskEnvironment (resolving paths against ProjectDirectory instead of CWD), the underlying ManifestUtil APIs remain unsafe for multithreaded execution.

Third-party task authors who use these APIs directly and want to migrate to -mt mode will encounter incorrect path resolution.

Context

From PR #13177 review discussion between @OvesN and @JanProvaznik:

@OvesN: Manifest.ResolveFiles() internally uses Directory.GetCurrentDirectory(). If a third-party task author uses this overload and wants to migrate their task to -mt mode, their paths would resolve against the wrong directory. Should we add a doc comment warning that it's not safe for multithreaded execution?

@JanProvaznik: Added comment for now, I think we can consider broader "MT-aware manifest library refactor" but I would rather focus on finalizing tasks rather than this refactor with questionable business value.

Affected APIs

  • Manifest.ResolveFiles() (parameterless overload) — uses Directory.GetCurrentDirectory() to resolve relative paths (Manifest.cs:220)
  • Util.WriteLog() — uses a static StreamWriter and logPath derived from environment at class init time
  • Various ManifestReader/ManifestWriter methods that accept string paths

Current State

  • A doc comment warning has been added to the parameterless ResolveFiles() overload.
  • All built-in MSBuild tasks using ManifestUtil already pass absolute paths (via TaskEnvironment.GetAbsolutePath()), so there is no correctness issue for built-in tasks.
  • The only caller of the unsafe parameterless ResolveFiles() is ResolveNativeReference, which is not migrated to IMultiThreadableTask and will be ejected to out-of-proc TaskHost in MT mode.

Potential Work

  • Add TaskEnvironment-accepting overloads to key ManifestUtil APIs, use them in our tasks
  • Discourage use of old APIs

Priority

Low — no built-in task is affected, and third-party manifest task migration is uncommon. This is tracked for completeness and to make our static analyzer clean.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area: TasksIssues impacting the tasks shipped in Microsoft.Build.Tasks.Core.dll.

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions