Skip to content

Commit 648f791

Browse files
[xabt] Make static fields readonly to improve MSBuild safety (#10359)
Fixes: #5996 This PR addresses static field usage in Xamarin.Android.Build.Tasks to improve MSBuild safety and reduce potential thread safety issues, as identified in the audit request. ## Problem The original issue identified 27 mutable static fields that could be problematic in MSBuild context where tasks might run in parallel or be cached inappropriately. Mutable static fields can cause: 1. Race conditions during parallel task execution 2. Unexpected state mutations between builds 3. Thread safety issues in IDE scenarios where SDK paths might change ## Solution After systematically auditing all static fields in the Build.Tasks assembly, I identified and fixed 13 static fields that should be immutable after initialization by adding the `readonly` modifier: **XML and Configuration Fields:** - `ManifestDocument.AndroidXmlNamespace` & `AndroidXmlToolsNamespace` - XML namespace constants - `ManifestDocument.androidNs` & `androidToolsNs` - Cached namespace references - `JavaObjectsXmlFile.settings` - XML writer settings - `SupportsGLTextureAttribute.mapping` - Manifest mapping configuration **Regex and Parsing Fields:** - `Aapt2Link.exraArgSplitRegEx` - Compiled regex for argument parsing - `ResourceIdentifier.validIdentifier` - Compiled regex for validation - `CheckForInvalidResourceFileNames.javaKeywords` - Java keyword array - `RtxtParser.knownTypes` - Set of known resource types **Utility Fields:** - `ManagedResourceParser.compareTuple` - Comparer instance - `RemoveUnknownFiles.IsWindows` & `PathUtil.IsWindows` - Platform detection flags ## Analysis of Remaining Static Fields The remaining static fields were analyzed and found to be appropriate: - `MonoAndroidHelper.SupportedVersions` & `AndroidSdk` - Set once per build by ResolveSdksTask following established MSBuild patterns - `AndroidLinkConfiguration.configurations` - Uses `ConditionalWeakTable` which is already thread-safe - `MonoAndroidHelper.uname` - Uses `Lazy<T>` with thread-safe initialization Co-authored-by: Jonathan Peppers <[email protected]>
1 parent 8daffb7 commit 648f791

File tree

10 files changed

+13
-13
lines changed

10 files changed

+13
-13
lines changed

src/Xamarin.Android.Build.Tasks/Mono.Android/SupportsGLTextureAttribute.Partial.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ namespace Android.App
1212
{
1313
partial class SupportsGLTextureAttribute
1414
{
15-
static ManifestDocumentElement<SupportsGLTextureAttribute> mapping = new ManifestDocumentElement<SupportsGLTextureAttribute> ("supports-gl-texture") {
15+
static readonly ManifestDocumentElement<SupportsGLTextureAttribute> mapping = new ManifestDocumentElement<SupportsGLTextureAttribute> ("supports-gl-texture") {
1616
{
1717
"Name",
1818
"name",

src/Xamarin.Android.Build.Tasks/Tasks/Aapt2Link.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ namespace Xamarin.Android.Tasks {
2020

2121
//aapt2 link -o resources.apk.bk --manifest Foo.xml --java . --custom-package com.infinitespace_studios.blankforms -R foo2 -v --auto-add-overlay
2222
public class Aapt2Link : Aapt2 {
23-
static Regex exraArgSplitRegEx = new Regex (@"[\""].+?[\""]|[\''].+?[\'']|[^ ]+", RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.Multiline);
23+
static readonly Regex exraArgSplitRegEx = new Regex (@"[\""].+?[\""]|[\''].+?[\'']|[^ ]+", RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.Multiline);
2424
public override string TaskPrefix => "A2L";
2525

2626
[Required]

src/Xamarin.Android.Build.Tasks/Tasks/CheckForInvalidResourceFileNames.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public class CheckForInvalidResourceFileNames : AndroidTask {
2424
Regex fileNameWithHyphenCheck = new Regex ("[^a-zA-Z0-9_.-]+", RegexOptions.Compiled);
2525

2626
// Source https://docs.oracle.com/javase/tutorial/java/nutsandbolts/_keywords.html
27-
static string [] javaKeywords = {
27+
static readonly string [] javaKeywords = {
2828
"abstract",
2929
"assert",
3030
"boolean",

src/Xamarin.Android.Build.Tasks/Tasks/RemoveUnknownFiles.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public class RemoveUnknownFiles : AndroidTask
1414
{
1515
public override string TaskPrefix => "RUF";
1616

17-
static bool IsWindows = Path.DirectorySeparatorChar == '\\';
17+
static readonly bool IsWindows = Path.DirectorySeparatorChar == '\\';
1818

1919
[Required]
2020
public ITaskItem[] Files { get; set; } = [];

src/Xamarin.Android.Build.Tasks/Utilities/JavaObjectsXmlFile.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ namespace Xamarin.Android.Tasks;
1414

1515
class JavaObjectsXmlFile
1616
{
17-
static XmlWriterSettings settings = new XmlWriterSettings {
17+
static readonly XmlWriterSettings settings = new XmlWriterSettings {
1818
Indent = true,
1919
NewLineOnAttributes = false,
2020
OmitXmlDeclaration = true,

src/Xamarin.Android.Build.Tasks/Utilities/ManagedResourceParser.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public int Compare((int Key, CodeMemberField Value) x, (int Key, CodeMemberField
3535
List<CodeTypeDeclaration> typeIds = new List<CodeTypeDeclaration> ();
3636
Dictionary<CodeMemberField, CodeMemberField []> arrayMapping = new Dictionary<CodeMemberField, CodeMemberField []> ();
3737
const string itemPackageId = "0x7f";
38-
static CompareTuple compareTuple = new CompareTuple ();
38+
static readonly CompareTuple compareTuple = new CompareTuple ();
3939

4040
XDocument publicXml;
4141

src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,13 @@ namespace Xamarin.Android.Tasks {
3131

3232
internal class ManifestDocument
3333
{
34-
public static XNamespace AndroidXmlNamespace = "http://schemas.android.com/apk/res/android";
35-
public static XNamespace AndroidXmlToolsNamespace = "http://schemas.android.com/tools";
34+
public static readonly XNamespace AndroidXmlNamespace = "http://schemas.android.com/apk/res/android";
35+
public static readonly XNamespace AndroidXmlToolsNamespace = "http://schemas.android.com/tools";
3636

3737
const int maxVersionCode = 2100000000;
3838

39-
static XNamespace androidNs = AndroidXmlNamespace;
40-
static XNamespace androidToolsNs = AndroidXmlToolsNamespace;
39+
static readonly XNamespace androidNs = AndroidXmlNamespace;
40+
static readonly XNamespace androidToolsNs = AndroidXmlToolsNamespace;
4141
static readonly XName versionCodeAttributeName = androidNs + "versionCode";
4242

4343
XDocument doc;

src/Xamarin.Android.Build.Tasks/Utilities/PathUtil.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public static class PathUtil
1515

1616
internal const char VolumeSeparatorChar = ':';
1717

18-
static bool IsWindows = Path.DirectorySeparatorChar == '\\';
18+
static readonly bool IsWindows = Path.DirectorySeparatorChar == '\\';
1919
// Adapted from CoreFX sources
2020
public static string GetRelativePath(string relativeTo, string path, StringComparison comparisonType = StringComparison.OrdinalIgnoreCase)
2121
{

src/Xamarin.Android.Build.Tasks/Utilities/ResourceIdentifier.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public class ResourceIdentifier {
3232
private const string Identifier = IdentifierStartCharacter + "(" + IdentifierPartCharacter + ")";
3333

3434
// We use [^ ...] to detect any character that is NOT a match.
35-
static Regex validIdentifier = new Regex ($"[^{Identifier}]", RegexOptions.Compiled);
35+
static readonly Regex validIdentifier = new Regex ($"[^{Identifier}]", RegexOptions.Compiled);
3636

3737
public static string CreateValidIdentifier (string identifier)
3838
{

src/Xamarin.Android.Build.Tasks/Utilities/RtxtParser.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public class RtxtParser {
7171
TaskLoggingHelper? log;
7272
Dictionary<string, string>? map;
7373

74-
public static HashSet<string> knownTypes = new HashSet<string> () {
74+
public static readonly HashSet<string> knownTypes = new HashSet<string> () {
7575
"anim",
7676
"animator",
7777
"attr",

0 commit comments

Comments
 (0)