From 5d128fe0c647af075df538b018ced344f4b389dc Mon Sep 17 00:00:00 2001 From: Ahmet Ibrahim Aksoy Date: Wed, 7 Sep 2022 16:33:56 +0300 Subject: [PATCH 1/5] Update: Fix Race Condition Issue --- .../System.Net.WebProxy/src/System/Net/WebProxy.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs b/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs index e76ae87717ad42..2b56a1f1371e69 100644 --- a/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs +++ b/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs @@ -130,7 +130,6 @@ private void UpdateRegexList() Regex[]? regexBypassList = null; if (_bypassList is ChangeTrackingArrayList bypassList) { - bypassList.IsChanged = false; if (bypassList.Count > 0) { regexBypassList = new Regex[bypassList.Count]; @@ -139,9 +138,14 @@ private void UpdateRegexList() regexBypassList[i] = new Regex((string)bypassList[i]!, RegexOptions.IgnoreCase | RegexOptions.CultureInvariant); } } - } - _regexBypassList = regexBypassList; + _regexBypassList = regexBypassList; + bypassList.IsChanged = false; + } + else + { + _regexBypassList = null; + } } private bool IsMatchInBypassList(Uri input) From d6329ff138ab2342685166a22ffa9774c7fd8759 Mon Sep 17 00:00:00 2001 From: Ahmet Ibrahim Aksoy Date: Wed, 7 Sep 2022 19:33:28 +0300 Subject: [PATCH 2/5] Update: Review changes --- .../src/System/Net/WebProxy.cs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs b/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs index 2b56a1f1371e69..6aee0dc4866235 100644 --- a/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs +++ b/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs @@ -8,6 +8,7 @@ using System.Globalization; using System.Runtime.Serialization; using System.Text.RegularExpressions; +using System.Threading; namespace System.Net { @@ -127,9 +128,9 @@ public bool UseDefaultCredentials private void UpdateRegexList() { - Regex[]? regexBypassList = null; if (_bypassList is ChangeTrackingArrayList bypassList) { + Regex[]? regexBypassList = null; if (bypassList.Count > 0) { regexBypassList = new Regex[bypassList.Count]; @@ -223,7 +224,20 @@ public ChangeTrackingArrayList() { } public ChangeTrackingArrayList(ICollection c) : base(c) { } - public bool IsChanged { get; set; } + private volatile bool _isChanged; + + public bool IsChanged + { + get + { + return _isChanged; + } + + set + { + _isChanged = value; + } + } // Override the methods that can add, remove, or change the regexes in the bypass list. // Methods that only read (like CopyTo, BinarySearch, etc.) and methods that reorder From 3bd7522c76d8b9b14ea37eb767717222f0b9aee1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ahmet=20=C4=B0brahim=20AKSOY?= Date: Wed, 7 Sep 2022 19:37:44 +0300 Subject: [PATCH 3/5] Update: Review change Co-authored-by: Miha Zupan --- .../src/System/Net/WebProxy.cs | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs b/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs index 6aee0dc4866235..0fdc5cfbabc2d9 100644 --- a/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs +++ b/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs @@ -224,20 +224,7 @@ public ChangeTrackingArrayList() { } public ChangeTrackingArrayList(ICollection c) : base(c) { } - private volatile bool _isChanged; - - public bool IsChanged - { - get - { - return _isChanged; - } - - set - { - _isChanged = value; - } - } + public volatile bool IsChanged; // Override the methods that can add, remove, or change the regexes in the bypass list. // Methods that only read (like CopyTo, BinarySearch, etc.) and methods that reorder From 1ea41d9cf6c7bb73cb245581b4b3cb38087fe219 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ahmet=20=C4=B0brahim=20AKSOY?= Date: Thu, 8 Sep 2022 07:00:11 +0300 Subject: [PATCH 4/5] Update: Add comment to volatile field Co-authored-by: Stephen Toub --- src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs b/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs index 0fdc5cfbabc2d9..2c8cb1374cfc4f 100644 --- a/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs +++ b/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs @@ -224,6 +224,9 @@ public ChangeTrackingArrayList() { } public ChangeTrackingArrayList(ICollection c) : base(c) { } + // While this type isn't intended to mutated concurrently with reads, non-concurrent updates + // to the list might result in lazy initialization, and it's possible concurrent requests could race + // to trigger that initialization. public volatile bool IsChanged; // Override the methods that can add, remove, or change the regexes in the bypass list. From 489303f47ac8979a6e0bf27f42deb7af6d00cd65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ahmet=20=C4=B0brahim=20AKSOY?= Date: Thu, 8 Sep 2022 18:28:03 +0300 Subject: [PATCH 5/5] Update src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs Co-authored-by: Karel Zikmund --- src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs b/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs index 2c8cb1374cfc4f..94b7e980745566 100644 --- a/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs +++ b/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs @@ -224,8 +224,8 @@ public ChangeTrackingArrayList() { } public ChangeTrackingArrayList(ICollection c) : base(c) { } - // While this type isn't intended to mutated concurrently with reads, non-concurrent updates - // to the list might result in lazy initialization, and it's possible concurrent requests could race + // While this type isn't intended to be mutated concurrently with reads, non-concurrent updates + // to the list might result in lazy initialization, and it's possible concurrent HTTP requests could race // to trigger that initialization. public volatile bool IsChanged;