[Type: Refactoring]
[Scope: src/Tizen.Applications.PackageManager]
[Priority: 🟡 Improvement]
[Lens: Performance, Clean Code, Coding Guidelines]
Observation
Install / uninstall / move requests with an event callback are tracked in three parallel static dictionaries keyed by the same requestId (src/Tizen.Applications.PackageManager/Tizen.Applications/PackageManager.cs:112-114):
private static Dictionary<int, RequestEventCallback> RequestCallbacks = new Dictionary<int, RequestEventCallback>();
private static Dictionary<int, SafePackageManagerRequestHandle> RequestHandles = new Dictionary<int, SafePackageManagerRequestHandle>();
private static Dictionary<int, int> RequestPackageCount = new Dictionary<int, int>();
They are populated in 4 places (InstallInternal:813-815 and 838-840, Uninstall:996-998, Move:1127-1129) and consumed/purged inside the native progress callback internalRequestEventCallback (:288-317):
if (RequestCallbacks.ContainsKey(id)) // lookup 1
{
try
{
RequestCallbacks[id](packageType, packageId, ...); // lookup 2
if (eventState == ... Completed || eventState == ... Failed)
{
RequestPackageCount[id] -= 1; // lookups 3+4 (get + set)
if (RequestPackageCount[id] < 1) // lookup 5
{
RequestHandles[id].Dispose(); // lookup 6
RequestHandles.Remove(id); // + 3 removes
RequestCallbacks.Remove(id);
RequestPackageCount.Remove(id);
}
}
}
catch (Exception e)
{
Log.Warn(LogTag, e.Message);
RequestHandles[id].Dispose();
...
}
}
Problem
- Unsynchronized cross-thread mutation (Coding Guidelines). The three dictionaries are mutated both from API threads (
Install/Uninstall/Move can be called from any thread, including concurrently) and from the event-dispatch path — with no lock. The sibling callback registries in the very same file (s_totalSizeInfoCallbackDict, and _packageManagerSizeInfoCallbackDict in Package.cs) are consistently guarded with lock — this trio is the inconsistency. Two concurrent Install calls racing on Dictionary.Add can corrupt internal state.
- Redundant hash operations on the progress-event path (Performance). Each progress event performs up to 6 lookups + 3 removes (9 hash operations) where 1
TryGetValue + 1 Remove suffice. Multi-package installs emit StartInstall/Downloading/Installing/Progress/Completed events per package, so this is a warm path during installation.
- Three-way structural duplication + fragile teardown (Clean Code). One logical record (callback, handle, remaining count) is spread across three maps that must always be added/removed in triplets; the
catch (Exception) recovery block duplicates the teardown a second time. A single record type removes the invariant-by-convention.
Proposed Improvement
Fuse the trio into one map guarded by one lock, and fetch the record once per event.
After (core shape)
private sealed class PackageRequest
{
public RequestEventCallback Callback;
public SafePackageManagerRequestHandle Handle;
public int RemainingPackageCount;
}
private static readonly Dictionary<int, PackageRequest> s_requests = new Dictionary<int, PackageRequest>();
private static readonly object s_requestLock = new object();
// registration (Install/Uninstall/Move):
lock (s_requestLock)
{
s_requests.Add(requestId, new PackageRequest { Callback = eventCallback, Handle = RequestHandle, RemainingPackageCount = packagePaths.Count });
}
// internalRequestEventCallback:
PackageRequest request;
lock (s_requestLock)
{
if (!s_requests.TryGetValue(id, out request))
{
return;
}
}
try
{
request.Callback(packageType, packageId, (PackageEventType)eventType, (PackageEventState)eventState, progress);
if (eventState == Interop.PackageManager.PackageEventState.Completed || eventState == Interop.PackageManager.PackageEventState.Failed)
{
if (--request.RemainingPackageCount < 1)
{
RemoveRequest(id, request); // single lock + Remove + Dispose helper
}
}
}
catch (Exception e)
{
Log.Warn(LogTag, e.Message);
RemoveRequest(id, request);
}
Target Files
src/Tizen.Applications.PackageManager/Tizen.Applications/PackageManager.cs
Expected Impact (Quantitative Metrics)
- Hash operations per progress event: up to 9 -> 2 (1
TryGetValue + 1 Remove on the final event only).
- Tracking dictionaries: 3 -> 1; triplet add/remove sites: 4x3 + 2x3 -> 6 single-record operations.
- Unsynchronized cross-thread dictionary mutation windows: eliminated (aligned with the locking discipline already used by the size-info registries in the same file).
- Teardown duplication in
internalRequestEventCallback: 2 copies -> 1 helper.
API Compatibility Check
- Public API signature change: none (all touched members are private statics).
- Behavior change: none on the success path; a user-callback exception still logs and tears down the request exactly as today.
- Tizen API Level floor: maintained (no new APIs).
Impact Scope
- 1 file, single assembly. Measured via rg: the three dictionaries are referenced in 16 locations, all within
PackageManager.cs; no other assembly touches them.
[Type: Refactoring]
[Scope: src/Tizen.Applications.PackageManager]
[Priority: 🟡 Improvement]
[Lens: Performance, Clean Code, Coding Guidelines]
Observation
Install / uninstall / move requests with an event callback are tracked in three parallel static dictionaries keyed by the same requestId (
src/Tizen.Applications.PackageManager/Tizen.Applications/PackageManager.cs:112-114):They are populated in 4 places (
InstallInternal:813-815 and 838-840,Uninstall:996-998,Move:1127-1129) and consumed/purged inside the native progress callbackinternalRequestEventCallback(:288-317):Problem
Install/Uninstall/Movecan be called from any thread, including concurrently) and from the event-dispatch path — with no lock. The sibling callback registries in the very same file (s_totalSizeInfoCallbackDict, and_packageManagerSizeInfoCallbackDictinPackage.cs) are consistently guarded withlock— this trio is the inconsistency. Two concurrentInstallcalls racing onDictionary.Addcan corrupt internal state.TryGetValue+ 1Removesuffice. Multi-package installs emitStartInstall/Downloading/Installing/Progress/Completedevents per package, so this is a warm path during installation.catch (Exception)recovery block duplicates the teardown a second time. A single record type removes the invariant-by-convention.Proposed Improvement
Fuse the trio into one map guarded by one lock, and fetch the record once per event.
After (core shape)
Target Files
src/Tizen.Applications.PackageManager/Tizen.Applications/PackageManager.csExpected Impact (Quantitative Metrics)
TryGetValue+ 1Removeon the final event only).internalRequestEventCallback: 2 copies -> 1 helper.API Compatibility Check
Impact Scope
PackageManager.cs; no other assembly touches them.