From ae63e51c4caf02802c8c1a34cc25584fb70f6a6d Mon Sep 17 00:00:00 2001 From: Ahmet Ibrahim Aksoy Date: Wed, 7 Sep 2022 16:33:56 +0300 Subject: [PATCH 1/4] 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 f8b9e44fccb07c587eac423905d5f1510c07e971 Mon Sep 17 00:00:00 2001 From: Ahmet Ibrahim Aksoy Date: Wed, 7 Sep 2022 19:33:28 +0300 Subject: [PATCH 2/4] 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 2147cd64d238981df7c9ffe9a09aec6ee99f21fd 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/4] 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 d54dd0d5c1b1babbc3cb4122911bcfb9a2ca2bad 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/4] 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.