Skip to content

Windows: Fix unbounded growth of stored scoped attributes #259

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Windows: Fix unbounded growth of stored scoped attributes #259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd0e25c
Windows: Fix unbounded growth of scoped attributes stored in PlayerPr…
melekr Oct 22, 2025
2e58aaf
Add Test coverage for scoped attribute persistence in Windows NativeC…
melekr Oct 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 148 additions & 20 deletions Runtime/Native/Windows/NativeClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ private string GetPluginDirectoryPath()
const string pluginDir = "Plugins";
return Path.Combine(Application.dataPath, pluginDir);
}

/// <summary>
/// Generate path to Crashpad handler binary
/// </summary>
Expand Down Expand Up @@ -367,12 +368,27 @@ internal static void CleanScopedAttributes()
{
return;
}
var attributes = JsonUtility.FromJson<ScopedAttributesContainer>(attributesJson);
foreach (var attributeKey in attributes.Keys)

try
{
var attributes = JsonUtility.FromJson<ScopedAttributesContainer>(attributesJson);
Copy link
Collaborator

@konraddysput konraddysput Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its our contract, and it should never break.

if (attributes?.Keys != null)
Copy link
Collaborator

@konraddysput konraddysput Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the if statement is invalid, then the catch should capture this. Since this is our contract and we require the people to follow it, why we should check it?

{
foreach (var attributeKey in attributes.Keys)
{
PlayerPrefs.DeleteKey(string.Format(ScopedAttributesPattern, attributeKey));
}
}
}
catch (Exception e)
{
PlayerPrefs.DeleteKey(string.Format(ScopedAttributesPattern, attributeKey));
Debug.LogWarning($"Backtrace Failed to parse scoped attributes for cleanup: {e.Message}");
}
finally
{
PlayerPrefs.DeleteKey(ScopedAttributeListKey);
PlayerPrefs.Save();
}
PlayerPrefs.DeleteKey(ScopedAttributeListKey);
}

internal static IDictionary<string, string> GetScopedAttributes()
Expand All @@ -382,8 +398,25 @@ internal static IDictionary<string, string> GetScopedAttributes()
{
return new Dictionary<string, string>();
}
var result = new Dictionary<string, string>();
var attributes = JsonUtility.FromJson<ScopedAttributesContainer>(attributesJson);

var result = new Dictionary<string, string>(StringComparer.Ordinal);
ScopedAttributesContainer attributes = null;

try
{
attributes = JsonUtility.FromJson<ScopedAttributesContainer>(attributesJson);
}
catch (Exception e)
{
Debug.LogWarning($"Backtrace Failed to parse scoped attributes at read: {e.Message}");
return result;
}

if (attributes?.Keys == null)
{
return result;
}

foreach (var attributeKey in attributes.Keys)
{
var value = PlayerPrefs.GetString(string.Format(ScopedAttributesPattern, attributeKey), string.Empty);
Expand Down Expand Up @@ -411,7 +444,7 @@ internal static IDictionary<string, string> GetScopedAttributes()
}

/// <summary>
/// Adds attributes to scoped registry and to native clietn
/// Adds attributes to scoped registry and to native client
/// </summary>
/// <param name="key">attribute key</param>
/// <param name="value">attribute value</param>
Expand All @@ -424,23 +457,103 @@ private void AddAttributes(string key, string value)
AddScopedAttribute(key, value);
}

/// <summary>Load a deduped set of scoped attribute keys from PlayerPrefs.</summary>
private HashSet<string> LoadScopedKeys()
{
var keys = new HashSet<string>(StringComparer.Ordinal);
var attributesJson = PlayerPrefs.GetString(ScopedAttributeListKey);
if (HasScopedAttributesEmpty(attributesJson))
{
try
{
var container = JsonUtility.FromJson<ScopedAttributesContainer>(attributesJson);
Copy link
Collaborator

@konraddysput konraddysput Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why you cannot deserialize array into hashset?

if (container?.Keys != null && container.Keys.Count > 0)
{
foreach (var k in container.Keys)
{
if (!string.IsNullOrEmpty(k))
{
keys.Add(k);
}
}
}
}
catch (Exception e)
{
Debug.LogWarning($"Backtrace Failed to parse scoped attributes list. {e.Message}");
keys.Clear();
}
}
return keys;
}

/// <summary>Persist a deduped list of keys back to PlayerPrefs.</summary>
private void SaveScopedKeys(HashSet<string> keys)
{
var container = new ScopedAttributesContainer
{
Keys = keys.OrderBy(k => k, StringComparer.Ordinal).ToList()
Copy link
Collaborator

@konraddysput konraddysput Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you need it? also the same order needs to be applied to values.

};
PlayerPrefs.SetString(ScopedAttributeListKey, JsonUtility.ToJson(container));
PlayerPrefs.Save();
}

/// <summary>Return the currently stored "value" for a scoped key or "null" if missing.</summary>
private string GetScopedValue(string key)
{
var prefsKey = string.Format(ScopedAttributesPattern, key);
return PlayerPrefs.HasKey(prefsKey) ? PlayerPrefs.GetString(prefsKey) : null;
}

/// <summary>Set the stored value for a scoped key.</summary>
private void SetScopedValue(string key, string value)
{
PlayerPrefs.SetString(string.Format(ScopedAttributesPattern, key), value ?? string.Empty);
}

/// <summary>
/// Adds dictionary of attributes to player prefs for windows crashes captured by unity crash handler
/// </summary>
/// <param name="atributes">Attributes</param>
/// <param name="attributes">Attributes</param>
internal void AddScopedAttributes(IDictionary<string, string> attributes)
{
if (!_configuration.SendUnhandledGameCrashesOnGameStartup)
{
return;
}
var attributesContainer = new ScopedAttributesContainer();
foreach (var attribute in attributes)
if (attributes == null || attributes.Count == 0)
{
attributesContainer.Keys.Add(attribute.Key);
PlayerPrefs.SetString(string.Format(ScopedAttributesPattern, attribute.Key), attribute.Value);
return;
}
PlayerPrefs.SetString(ScopedAttributeListKey, JsonUtility.ToJson(attributesContainer));

var keys = LoadScopedKeys();
bool listChanged = false;

foreach (var kv in attributes)
{
var key = kv.Key;
var value = kv.Value ?? string.Empty;

// skip when unchanged
var current = GetScopedValue(key);
if (current == null || !string.Equals(current, value, StringComparison.Ordinal))
{
SetScopedValue(key, value);
}

// deduplication
if (keys.Add(key))
{
listChanged = true;
}
}

if (listChanged)
{
SaveScopedKeys(keys);
}

PlayerPrefs.Save();
}

/// <summary>
Expand All @@ -454,14 +567,29 @@ private void AddScopedAttribute(string key, string value)
{
return;
}
var attributesJson = PlayerPrefs.GetString(ScopedAttributeListKey);
var attributes = HasScopedAttributesEmpty(attributesJson)
? JsonUtility.FromJson<ScopedAttributesContainer>(attributesJson)
: new ScopedAttributesContainer();
if (string.IsNullOrEmpty(key))
{
return;
}

var normalized = value ?? string.Empty;

// skip when unchanged
var current = GetScopedValue(key);
if (current != null && string.Equals(current, normalized, StringComparison.Ordinal))
{
return;
}

SetScopedValue(key, normalized);

var keys = LoadScopedKeys();
if (keys.Add(key))
{
SaveScopedKeys(keys);
}

attributes.Keys.Add(key);
PlayerPrefs.SetString(ScopedAttributeListKey, JsonUtility.ToJson(attributes));
PlayerPrefs.SetString(string.Format(ScopedAttributesPattern, key), value);
PlayerPrefs.Save();
}

private static bool HasScopedAttributesEmpty(string attributesJson)
Expand Down
66 changes: 64 additions & 2 deletions Tests/Runtime/Native/Windows/ScopedNativeAttributesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,27 @@ namespace Backtrace.Unity.Tests.Runtime.Native.Windows
{
public sealed class ScopedNativeAttributesTests
{
// PlayerPrefs key used by the Windows NativeClient to store the scoped list
private const string ScopedKeyList = "backtrace-scoped-attributes";
private const string ScopedValuePattern = "bt-{0}";

[TearDown]
public void Setup()
{
CleanLegacyAttributes();
NativeClient.CleanScopedAttributes();
PlayerPrefs.DeleteAll();
}

[Test]
public void FreshStartup_ShouldntIncludeAnyAttributeFromPlayerPrefs_AttributesAreEmpty()
{
var attributes = NativeClient.GetScopedAttributes();

Assert.IsEmpty(attributes);
}

[Test]
public void LegacyAttributesSupport_ShouldIncludeLegacyAttributesWhenScopedAttributesAreNotAvailable_AllLegacyAttributesArePresent()
{
string testVersion = "0.1.0";
Expand Down Expand Up @@ -82,7 +89,6 @@ public void NativeCrashUploadAttributesSetting_ShouldReadPlayerPrefsWithLegacyAt
[Test]
public void NativeCrashUploadAttributes_ShouldSetScopedAttributeViaNativeClientApi_AttributePresentsInScopedAttributes()
{

var configuration = ScriptableObject.CreateInstance<BacktraceConfiguration>();
configuration.SendUnhandledGameCrashesOnGameStartup = true;
const string testAttributeKey = "foo-key-bar-baz";
Expand All @@ -93,7 +99,6 @@ public void NativeCrashUploadAttributes_ShouldSetScopedAttributeViaNativeClientA
var scopedAttributes = NativeClient.GetScopedAttributes();

Assert.AreEqual(scopedAttributes[testAttributeKey], testAttributeValue);

}

[Test]
Expand All @@ -119,6 +124,63 @@ public void NativeCrashAttributesCleanMethod_ShouldCleanAllScopedAttribtues_Scop
Assert.IsNotEmpty(attributesBeforeCleanup);
}

[Test]
public void ScopedAttributes_ShouldNotDuplicateKeys_OnRepeatedAdds()
{
var configuration = ScriptableObject.CreateInstance<BacktraceConfiguration>();
configuration.SendUnhandledGameCrashesOnGameStartup = true;

const string k = "dup-key";
const string v = "v1";

var client = new NativeClient(configuration, null, new Dictionary<string, string>(), new List<string>());

// Add the same key/value multiple times
client.SetAttribute(k, v);
client.SetAttribute(k, v);
client.SetAttribute(k, v);

// Verify the stored list
var scoped = NativeClient.GetScopedAttributes();
Assert.IsTrue(scoped.ContainsKey(k));
Assert.AreEqual(v, scoped[k]);

// Inspect the JSON list to ensure one occurrence of the key
var json = PlayerPrefs.GetString(ScopedKeyList);
var occurrences = json.Split('"');
int count = 0;
foreach (var s in occurrences)
{
if (s == k) count++;
}
Assert.AreEqual(1, count, "Key should be stored once in the scoped key list.");
}

[Test]
public void ScopedAttributes_ShouldSkipWrites_WhenValueUnchanged()
{
var configuration = ScriptableObject.CreateInstance<BacktraceConfiguration>();
configuration.SendUnhandledGameCrashesOnGameStartup = true;

const string k = "stable-key";
const string v = "same-value";

var client = new NativeClient(configuration, null, new Dictionary<string, string>(), new List<string>());

// First write
client.SetAttribute(k, v);
var json1 = PlayerPrefs.GetString(ScopedKeyList);
var val1 = PlayerPrefs.GetString(string.Format(ScopedValuePattern, k));

// Second write with same value should be a no-op
client.SetAttribute(k, v);
var json2 = PlayerPrefs.GetString(ScopedKeyList);
var val2 = PlayerPrefs.GetString(string.Format(ScopedValuePattern, k));

Assert.AreEqual(json1, json2, "Scoped key list JSON should not change when value is unchanged.");
Assert.AreEqual(val1, val2, "Stored value should remain the same.");
}

private void CleanLegacyAttributes()
{
PlayerPrefs.DeleteKey(NativeClient.VersionKey);
Expand Down
Loading