Sanity fixes

CHANGELOG.md

@@ -1,4 +1,8 @@
 ## XX.XX.XX
+* Fixed public APIs (RecordEvent, RecordView, RecordException, SetLocation, DisableLocation, StartEvent, EndEvent, CancelEvent, AddCrashBreadCrumb, ChangeDeviceId, SetConsent) throwing a NullReferenceException when called before "Init"; they now safely no-op until the SDK is initialized.
+* Fixed the SDK constructing a new HttpClient for every request, which could exhaust sockets under sustained use; a single shared client with a request timeout is now reused.
+* Fixed the .NET Framework (net35/net45) networking path leaking the HTTP response stream and losing the real status code on 4xx/5xx responses.
+* Fixed unique-timestamp generation not being thread-safe, which could hand out duplicate timestamps when recording from multiple threads at once.
 * Added support for a Log Listener: a callback set at initialization via the new configuration option "SetLogListener" that receives every SDK log message and its level. It fires independently of the console logging flag, so SDK logs can be captured in release builds without printing to the console.
 * Added support for SDK Health Checks: once per initialization, right after the SDK Behavior Settings fetch, the SDK sends a non-queued direct request to "/i" reporting internal warning/error log counts and the last failed request's status/body. This can be turned off with the new configuration option "DisableHealthCheck()".
 * Added support for SDK Behavior Settings (Server Config), enabled by default:

countlyCommon/TestingRelated/PreInitSafetyTests.cs

@@ -0,0 +1,92 @@
+using System;
+using System.Collections.Generic;
+using System.Threading.Tasks;
+using CountlySDK;
+using Xunit;
+using static CountlySDK.CountlyCommon.CountlyBase;
+
+namespace TestProject_common
+{
+    /// <summary>
+    /// Public entry points must not crash the host app when they are called before Init
+    /// (a common race: an async Init running while telemetry calls arrive). In the
+    /// never-initialized state Configuration is null, and several guards used to dereference
+    /// Configuration.backendMode before the IsInitialized() check, throwing a
+    /// NullReferenceException. These tests pin the "no throw, no-op" contract.
+    /// </summary>
+    public class PreInitSafetyTests : IDisposable
+    {
+        public PreInitSafetyTests()
+        {
+            TestHelper.CleanDataFiles();
+            Countly.Halt();
+            TestHelper.CleanDataFiles();
+            // Simulate the genuine never-initialized state: Halt clears ServerUrl/AppKey but
+            // intentionally leaves Configuration set, so we null it explicitly here.
+            Countly.Instance.Configuration = null;
+        }
+
+        public void Dispose()
+        {
+        }
+
+        [Fact]
+        public async Task RecordEvent_BeforeInit_ReturnsFalseWithoutThrowing()
+        {
+            bool result = await Countly.RecordEvent("pre_init_event");
+            Assert.False(result);
+        }
+
+        [Fact]
+        public async Task RecordView_BeforeInit_ReturnsFalseWithoutThrowing()
+        {
+            bool result = await Countly.Instance.RecordView("pre_init_view");
+            Assert.False(result);
+        }
+
+        [Fact]
+        public async Task RecordException_BeforeInit_ReturnsFalseWithoutThrowing()
+        {
+            bool result = await Countly.RecordException("err", "stack", null, false);
+            Assert.False(result);
+        }
+
+        [Fact]
+        public async Task SetLocation_BeforeInit_ReturnsFalseWithoutThrowing()
+        {
+            bool result = await Countly.Instance.SetLocation("12.34,56.78");
+            Assert.False(result);
+        }
+
+        [Fact]
+        public async Task DisableLocation_BeforeInit_ReturnsFalseWithoutThrowing()
+        {
+            bool result = await Countly.Instance.DisableLocation();
+            Assert.False(result);
+        }
+
+        [Fact]
+        public void StartEvent_BeforeInit_DoesNotThrow()
+        {
+            Countly.Instance.StartEvent("pre_init_timed");
+        }
+
+        [Fact]
+        public void AddCrashBreadCrumb_BeforeInit_DoesNotThrow()
+        {
+            Countly.Instance.AddCrashBreadCrumb("crumb");
+        }
+
+        [Fact]
+        public async Task ChangeDeviceId_BeforeInit_DoesNotThrow()
+        {
+            await Countly.Instance.ChangeDeviceId("new_device_id");
+        }
+
+        [Fact]
+        public async Task SetConsent_BeforeInit_DoesNotThrow()
+        {
+            await Countly.Instance.SetConsent(new Dictionary<ConsentFeatures, bool> { { ConsentFeatures.Events, true } });
+        }
+    }
+}

countlyCommon/TestingRelated/TimeHelperTests.cs

@@ -0,0 +1,46 @@
+using System.Collections.Concurrent;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using CountlySDK.Helpers;
+using Xunit;
+
+namespace TestProject_common
+{
+    public class TimeHelperTests
+    {
+        /// <summary>
+        /// GetUniqueUnixTime must hand out a strictly-unique value on every call, even when
+        /// hammered from many threads at once. The counter is a read-modify-write on a shared
+        /// field, so without synchronization two threads can observe the same value and both
+        /// return it. This drives concurrent callers hard and asserts every value is unique.
+        /// </summary>
+        [Fact]
+        public void GetUniqueUnixTime_UnderConcurrency_ReturnsAllUniqueValues()
+        {
+            TimeHelper timeHelper = new TimeHelper();
+            const int threadCount = 8;
+            const int perThread = 20000;
+
+            ConcurrentQueue<long> results = new ConcurrentQueue<long>();
+
+            using (Barrier barrier = new Barrier(threadCount)) {
+                Task[] tasks = new Task[threadCount];
+                for (int t = 0; t < threadCount; t++) {
+                    tasks[t] = Task.Run(() => {
+                        barrier.SignalAndWait();
+                        for (int i = 0; i < perThread; i++) {
+                            results.Enqueue(timeHelper.GetUniqueUnixTime());
+                        }
+                    });
+                }
+
+                Task.WaitAll(tasks);
+            }
+
+            int total = threadCount * perThread;
+            int distinct = results.Distinct().Count();
+            Assert.Equal(total, distinct);
+        }
+    }
+}

countlyCommon/countlyCommon/CountlyBase.cs

@@ -232,7 +232,7 @@
                 ModuleBackendMode backendMode = moduleBackendMode;
                 if (backendMode != null) {
                     backendMode.OnTimer();
                     Upload();
                 }
                 return;
             }
@@ -527,7 +527,7 @@
         /// <returns></returns>
         public void StartEvent(string key)
         {
-            if (Configuration.backendMode) {
+            if (Configuration?.backendMode ?? false) {
                 UtilityHelper.CountlyLogging("[CountlyBase] StartEvent, Backend Mode enabled, returning");
                 return;
             }
@@ -565,7 +565,7 @@
         /// <returns></returns>
         public void CancelEvent(string key)
         {
-            if (Configuration.backendMode) {
+            if (Configuration?.backendMode ?? false) {
                 UtilityHelper.CountlyLogging("[CountlyBase] CancelEvent, Backend Mode enabled, returning");
                 return;
             }
@@ -607,7 +607,7 @@
         /// <returns></returns>
         public async Task EndEvent(string key, Segmentation segmentation = null, int count = 1, double? sum = 0)
         {
-            if (Configuration.backendMode) {
+            if (Configuration?.backendMode ?? false) {
                 UtilityHelper.CountlyLogging("[CountlyBase] EndEvent, Backend Mode enabled, returning");
                 return;
             }
@@ -710,14 +710,14 @@
         /// <returns>True if event is uploaded successfully, False - queued for delayed upload</returns>
         public static Task<bool> RecordEvent(string Key, int Count, double? Sum, double? Duration, Segmentation Segmentation)
         {
-            if (Countly.Instance.Configuration.backendMode) {
+            if (Countly.Instance.Configuration?.backendMode ?? false) {
                 UtilityHelper.CountlyLogging("[CountlyBase] RecordEvent, Backend Mode enabled, returning false");
-                return Task.Factory.StartNew(() => { return false; });
+                return BoolTask(false);
             }
 
             if (!Countly.Instance.IsInitialized()) {
                 UtilityHelper.CountlyLogging("SDK must initialized before calling 'RecordEvent'");
-                return Task.Factory.StartNew(() => { return false; });
+                return BoolTask(false);
             }
 
             CountlyConfig config = Countly.Instance.Configuration;
@@ -976,7 +976,7 @@
         /// <returns>True if exception successfully uploaded, False - queued for delayed upload</returns>
         public static async Task<bool> RecordException(string error, string stackTrace, Dictionary<string, string> customInfo, bool unhandled)
         {
-            if (Countly.Instance.Configuration.backendMode) {
+            if (Countly.Instance.Configuration?.backendMode ?? false) {
                 UtilityHelper.CountlyLogging("[CountlyBase] RecordException, Backend Mode enabled, returning false");
                 return false;
             }
@@ -1337,7 +1337,7 @@
         /// <param name="log">log string</param>
         public void AddCrashBreadCrumb(string breadCrumb)
         {
-            if (Configuration.backendMode) {
+            if (Configuration?.backendMode ?? false) {
                 UtilityHelper.CountlyLogging("[CountlyBase] AddCrashBreadCrumb, Backend Mode enabled, returning");
                 return;
             }
@@ -1392,7 +1392,7 @@
         public async Task<bool> SetLocation(string gpsLocation, string ipAddress = null, string country_code = null, string city = null)
         {
 
-            if (Configuration.backendMode) {
+            if (Configuration?.backendMode ?? false) {
                 UtilityHelper.CountlyLogging("[CountlyBase] SetLocation, Backend Mode enabled, returning false");
                 return false;
             }
@@ -1490,7 +1490,7 @@
 
         public async Task<bool> DisableLocation()
         {
-            if (Configuration.backendMode) {
+            if (Configuration?.backendMode ?? false) {
                 UtilityHelper.CountlyLogging("[CountlyBase] DisableLocation, Backend Mode enabled, returning false");
                 return false;
             }
@@ -1517,6 +1517,16 @@
             return false;
         }
 
+        // Returns an already-completed Task with the given value. net35 has no Task.FromResult,
+        // so we complete a TaskCompletionSource explicitly instead of scheduling a thread-pool
+        // work item just to return a constant.
+        private static Task<bool> BoolTask(bool value)
+        {
+            TaskCompletionSource<bool> tcs = new TaskCompletionSource<bool>();
+            tcs.SetResult(value);
+            return tcs.Task;
+        }
+
         /// <summary>
         /// Drops queued requests older than the server-configured 'dort' limit (in hours).
         /// The request age is read from its embedded 'timestamp' parameter; 0 disables the feature.
@@ -1543,7 +1553,7 @@
                 }
                 if (keptRequests.Count != originalCount) {
                     StoredRequests = keptRequests;
                     SaveStoredRequests();
                 }
             }
         }
@@ -1586,7 +1596,7 @@
 
                 StoredRequests.Enqueue(sr);
                 if (!Configuration.backendMode) {
                     SaveStoredRequests();
                 } else {
                     UtilityHelper.CountlyLogging("[CountlyBase] AddRequest, Backend mode enabled, request storage disabled");
                 }
@@ -1838,7 +1848,7 @@
         public async Task ChangeDeviceId(string newDeviceId, bool serverSideMerge = false)
         {
 
-            if (Configuration.backendMode) {
+            if (Configuration?.backendMode ?? false) {
                 UtilityHelper.CountlyLogging("[CountlyBase] ChangeDeviceId, Backend Mode enabled, returning");
                 return;
             }
@@ -1944,7 +1954,7 @@
 
         public async Task SetConsent(Dictionary<ConsentFeatures, bool> consentChanges)
         {
-            if (Configuration.backendMode) {
+            if (Configuration?.backendMode ?? false) {
                 UtilityHelper.CountlyLogging("[CountlyBase] SetConsent, Backend Mode enabled, returning");
                 return;
             }
@@ -2091,7 +2101,7 @@
         /// <returns></returns>
         public async Task<bool> RecordView(string viewName)
         {
-            if (Configuration.backendMode) {
+            if (Configuration?.backendMode ?? false) {
                 UtilityHelper.CountlyLogging("[CountlyBase] RecordView, Backend Mode enabled, returning false");
                 return false;
             }

countlyCommon/countlyCommon/Helpers/TimeHelper.cs

@@ -64,6 +64,9 @@ internal static TimeInstant Get(long timestampInMillis)
 
         //variable to hold last used timestamp
         private long _lastMilliSecTimeStamp = 0;
+        //guards the read-modify-write on _lastMilliSecTimeStamp so concurrent callers
+        //(events/views recorded from multiple threads) never receive the same value
+        private readonly object timeLock = new object();
 
         internal TimeHelper() { }
 
@@ -82,15 +85,17 @@ public long ToUnixTime(DateTime date)
 
         public long GetUniqueUnixTime()
         {
-            long calculatedMillis = ToUnixTime(DateTime.Now.ToUniversalTime());
+            lock (timeLock) {
+                long calculatedMillis = ToUnixTime(DateTime.Now.ToUniversalTime());
 
-            if (_lastMilliSecTimeStamp >= calculatedMillis) {
-                ++_lastMilliSecTimeStamp;
-            } else {
-                _lastMilliSecTimeStamp = calculatedMillis;
-            }
+                if (_lastMilliSecTimeStamp >= calculatedMillis) {
+                    ++_lastMilliSecTimeStamp;
+                } else {
+                    _lastMilliSecTimeStamp = calculatedMillis;
+                }
 
-            return _lastMilliSecTimeStamp;
+                return _lastMilliSecTimeStamp;
+            }
         }
 
         public TimeInstant GetUniqueInstant()

countlyCommon/countlyCommon/Server/ApiBase.cs

@@ -115,7 +115,7 @@ protected async Task<RequestResult> CallJob(string address, string requestData,
             if (requestData.StartsWith("/i?")) { // for migrating old requests
                 requestData = requestData.Replace("/i?", "");
             }
-            requestData = AddChekcsum(requestData);
+            requestData = AddChecksum(requestData);
             UtilityHelper.CountlyLogging(string.Format("[ApiBase] CallJob, address: [{0}], endpoint: [{1}] requestData: [{2}]", address, endpoint, requestData));
 
             try {
@@ -141,7 +141,7 @@ protected async Task<RequestResult> CallJob(string address, string requestData,
             return await tcs.Task;
         }
 
-        private string AddChekcsum(string data)
+        private string AddChecksum(string data)
         {
             if (tamperingProtectionSalt == null || tamperingProtectionSalt.Length == 0) {
                 return data;

countlyCommon/countlyCommon/Server/Responses/RequestResult.cs

@@ -20,7 +20,9 @@ public bool IsSuccess()
 
             try {
                 return JObject.Parse(responseText).ContainsKey("result");
-            } catch (Exception e) { }
+            } catch (Exception ex) {
+                CountlySDK.Helpers.UtilityHelper.CountlyLogging("[RequestResult] IsSuccess, could not parse response as JSON: " + ex.Message);
+            }
 
             return false;
         }

net35/Countly/Server/Api.cs

@@ -79,10 +79,27 @@ protected override async Task<RequestResult> RequestAsync(string address, String
                     }
                 }
 
-                var response = (HttpWebResponse)request.GetResponse();
-                requestResult.responseCode = (int)response.StatusCode;
-                requestResult.responseText = new StreamReader(response.GetResponseStream()).ReadToEnd();
+                using (var response = (HttpWebResponse)request.GetResponse())
+                using (var responseStream = response.GetResponseStream())
+                using (var reader = new StreamReader(responseStream)) {
+                    requestResult.responseCode = (int)response.StatusCode;
+                    requestResult.responseText = reader.ReadToEnd();
+                }
 
+                return requestResult;
+            } catch (WebException wex) {
+                // GetResponse() throws on 4xx/5xx. Recover the real status code and body from
+                // the exception's response instead of leaving responseCode at -1.
+                UtilityHelper.CountlyLogging("Encountered a WebException while making a POST request, " + wex.ToString());
+                HttpWebResponse errorResponse = wex.Response as HttpWebResponse;
+                if (errorResponse != null) {
+                    using (errorResponse)
+                    using (var errorStream = errorResponse.GetResponseStream())
+                    using (var errorReader = new StreamReader(errorStream)) {
+                        requestResult.responseCode = (int)errorResponse.StatusCode;
+                        requestResult.responseText = errorReader.ReadToEnd();
+                    }
+                }
                 return requestResult;
             } catch (Exception ex) {
                 UtilityHelper.CountlyLogging("Encountered a exception while making a POST request, " + ex.ToString());

net45/Countly/Server/Api.cs

@@ -80,10 +80,27 @@ protected override async Task<RequestResult> RequestAsync(string address, String
                     }
                 }
 
-                var response = (HttpWebResponse)request.GetResponse();
-                requestResult.responseCode = (int)response.StatusCode;
-                requestResult.responseText = new StreamReader(response.GetResponseStream()).ReadToEnd();
+                using (var response = (HttpWebResponse)request.GetResponse())
+                using (var responseStream = response.GetResponseStream())
+                using (var reader = new StreamReader(responseStream)) {
+                    requestResult.responseCode = (int)response.StatusCode;
+                    requestResult.responseText = reader.ReadToEnd();
+                }
 
+                return requestResult;
+            } catch (WebException wex) {
+                // GetResponse() throws on 4xx/5xx. Recover the real status code and body from
+                // the exception's response instead of leaving responseCode at -1.
+                UtilityHelper.CountlyLogging("Encountered a WebException while making a POST request, " + wex.ToString());
+                HttpWebResponse errorResponse = wex.Response as HttpWebResponse;
+                if (errorResponse != null) {
+                    using (errorResponse)
+                    using (var errorStream = errorResponse.GetResponseStream())
+                    using (var errorReader = new StreamReader(errorStream)) {
+                        requestResult.responseCode = (int)errorResponse.StatusCode;
+                        requestResult.responseText = errorReader.ReadToEnd();
+                    }
+                }
                 return requestResult;
             } catch (Exception ex) {
                 UtilityHelper.CountlyLogging("Encountered a exception while making a POST request, " + ex.ToString());