Multithreading issue when setting a key/value in ApplicationData.Current.LocalSettings

[JosHuybrighs]

Describe the bug

I am porting a UWP/win32 desktop bridge app to WinUI 3 Desktop (MSIX packaged, WinUI 3 v1.0 stable) and are getting the following exception when assigning a composite value to ApplicationData.Current.LocalSettings from within a background task.

The application called an interface that was marshalled for a different thread. (Exception from HRESULT: 0x8001010E (RPC_E_WRONG_THREAD))

The background task that writes the composite value does this by calling a singleton method AppSettings.Instance.UpdateTaskBackupResult(result), which internally uses a semaphore to protect against concurrent access. The part of this method that writes to local settings looks as follows:

ApplicationDataCompositeValue composite = new ApplicationDataCompositeValue();
composite["TaskId"] = result.TaskId;
composite["LastStatus"] = Convert.ToInt32(result.LastStatus);
.. some other 20 keys are assigned
// Serialize subtask results to JSON
DataContractJsonSerializer js = new DataContractJsonSerializer(typeof(List<SubTaskBackupResult>));
using (MemoryStream ms = new MemoryStream())
{
   js.WriteObject(ms, result.SubTaskResults);
   ms.Position = 0;
   using (StreamReader sr = new StreamReader(ms))
   {
      string json = sr.ReadToEnd();
      composite["SubTaskResults"] = json;
      sr.Close();
      ms.Close();
   }
}
ApplicationData.Current.LocalSettings.Values[key] = composite;

Immediately before creating the background task the UI thread also writes to LocalSettings, like so:

.. some code
AppSettings.Instance.UpdateTaskBackupResult(result);
.. some code
var t = Task.Run(async () =>
{
   // Tell ExecTasksRequestHandler to run
   await _execTasksHandler.RunAsync(taskRequests, _isBackgroundTaskRequest, isFirstSchedularRun, _execTasksCancelTokenSource.Token);

   // Finished
   IsBackupRunning = false;
});

I know that writing to local settings is somehow (don't know the details) virtualized in Windows but I am not aware of any multi-threading issues.

Did something change with Windows App SDK 1.0?
I am not sure but just recently are using 'v1.0 stable', instead of 'v1.0 preview 3' and have not seen this happening before.

Steps to reproduce the bug

Create WinUI 3 Desktop app.
Write to local settings from within a UI trigger.
Within the same trigger start a background task and write to local settings there as well.

Expected behavior

No response

Screenshots

No response

NuGet package version

1.0.0

Packaging type

Packaged (MSIX)

Windows version

Windows 10 version 21H1 (19043, May 2021 Update)

IDE

Visual Studio 2019

Additional context

No response

[DrusTheAxe]

which internally uses a semaphore to protect against concurrent access.

Protects how? Blocking threads with native mutex/etc can intersect (badly) with COM reentrancy. Does this use CoWaitForMultipleObjects or equivalent COM reentrancy savvy APIs?

Could you share this semaphoring singleton code?

[JosHuybrighs]

@DrusTheAxe Thanks for the reply. The singleton and the code that is using the semaphore looks as follows:

public class AppSettings
{
	static AppSettings sInstance;

	public static AppSettings Instance
	{
		get
		{
			if (sInstance == null)
			{
				sInstance = new AppSettings();
			}
			return sInstance;
		}
	}

	SemaphoreSlim _settingsSemaphore = new SemaphoreSlim(1);
	
	public void UpdateTaskBackupResult(TaskBackupResult result)
	{
		_settingsSemaphore.Wait();
		// Find index of the taskID
		var entry = TaskBackupResults.FirstOrDefault(t => t.TaskId == result.TaskId);
		if (entry != null)
		{
			int idx = TaskBackupResults.IndexOf(entry);
			TaskBackupResults[idx] = result;
			// Write "TaskResult-xx" key with result as a composite value
			SaveTaskBackupResultToSettings($"TaskResult-{idx}", result);
		}
		_settingsSemaphore.Release();
	}
	
	void SaveTaskBackupResultToSettings(string key, TaskBackupResult result)
	{
		try
		{
			ApplicationDataCompositeValue composite = new ApplicationDataCompositeValue();
			composite["TaskId"] = result.TaskId;
			composite["LastStatus"] = Convert.ToInt32(result.LastStatus);
			composite["Status"] = Convert.ToInt32(result.Status);
			// etc..
			composite["LastRunSimulated"] = result.LastRunSimulated;

			// Serialize subtask results to JSON
			DataContractJsonSerializer js = new DataContractJsonSerializer(typeof(List<SubTaskBackupResult>));
			using (MemoryStream ms = new MemoryStream())
			{
				js.WriteObject(ms, result.SubTaskResults);
				ms.Position = 0;
				using (StreamReader sr = new StreamReader(ms))
				{
					string json = sr.ReadToEnd();
					composite["SubTaskResults"] = json;
					sr.Close();
					ms.Close();
				}
			}

			ApplicationData.Current.LocalSettings.Values[key] = composite;
		}
		catch (Exception e)
		{  }
	}
	
}

The app is written in c# and uses the System.Threading api that is available there for things like semaphores and mutexes.
It uses a semaphore because there is only 1 process, but it can have multiple threads. I am not aware whether SemaphoreSlim can deal with COM reentrance or not. I assume it should.

What I have seen is that when I remove the AppSettings.Instance.UpdateTaskBackupResult() call in the UI thread right before starting the task (using Task.Run) and thus will only write to local settings from within the background task, that the exception doesn't occur anymore.
When I do the settings update also in the UI thread before starting the task, the issue re-appears in about half of the test cases. So it seems to be associated with how fast the background thread kicks in.
It seems that there is some delayed operation going on before the data is actually written in local settings and that this delay has some influence on the error coming up or not.

[DrusTheAxe]

How's the thread initialized for COM? STA and COM reentrancy tend to mix poorly.

when I remove the AppSettings.Instance.UpdateTaskBackupResult() call in the UI thread

UI threads tend to be STA.

It's unclear how UpdateTaskBackupResult is called, thus hard to say more.

I am not aware whether SemaphoreSlim can deal with COM reentrance or not. I assume it should.

I'd be wary making that assumption. Per the docs

The SemaphoreSlim is a lightweight alternative to the Semaphore class that doesn't use Windows kernel semaphores. Unlike the Semaphore class, the SemaphoreSlim class doesn't support named system semaphores. You can use it as a local semaphore only. The SemaphoreSlim class is the recommended semaphore for synchronization within a single app.

It doesn't say how it's implemented (other than not using a kernel Semaphore object) but it sounds similar to SRWLocks i.e. some form of CAS 'lockless' implementation. Certainly no hint how it interacts (if at all) with COM, STA and/or a message pump.

You're using the semaphore as a mutex (_settingsSemaphore = new SemaphoreSlim(1);). Why not use Mutex? Or some other C# mechanism that gets you the desired throttling to 1 caller at a time? More important, something that doesn't run afoul of COM reentrancy

@brialmsft @BenJKuhn @Scottj1s @jonwis do you know current best practices for this scenario?

[JosHuybrighs]

@DrusTheAxe Thanks for the reply.

I have been investigating this issue a bit more and changed the shared access protection to a Mutex, but it still shows the same marshalling exception.
Some observations:

• When I don't update the app settings from within the UI thread the exception does not appear.
• When I do an update from the UI thread immediately before starting the background thread the "marshalling" exception appears from within in the background thread even when using a Mutex instead of a SemaphoreSlim.

I am pretty sure that there has to be an issue with the way a composite value is written to local settings in the latest v1 Windows App SDK.
What I see for instance is:

• When not doing the appsettings update in the UI thread that there is a error/warning output on the Visual Studio console immedialy after the call
  ApplicationData.Current.LocalSettings.Values[key] = composite;
  The error/warning is:
  onecore\base\appmodel\statemanager\apiset\lib\stateatom.cpp(593)\kernelbase.dll!00007FFECAA79CFC: (caller: 00007FFECAA79C34) ReturnHr(21) tid(5490) 8007007A The data area passed to a system call is too small.
• When the settings are also updated from the UI thread the above error/warning does not appear but I get the marshalling exception. I think it is even coming before the composite value is assigned to the key.

I must also say that the same code is being in use for already 2 years on an existing UWP/win32 desktop bridge app in the Windows Store (SyncFolder) without causing any problems.
It is now coming up with the porting of the app to a Windows App SDK v1 WinUI-Desktop app.

[JosHuybrighs]

@DrusTheAxe
I have created a basic Visual Studio WinUI project using the Visual Studio 2019 template for Windows App SDK to demonstrate the issue reported above.
There is a simple button in MainWindow.cs which does the following after being pressed:

        private void myButton_Click(object sender, RoutedEventArgs e)
        {
            myButton.Content = "Clicked";
            TaskBackupResult result = new TaskBackupResult("1234567890abcdefg");
            AppSettings.Instance.UpdateTaskBackupResult(result);
            var t = Task.Run(() =>
            {
                // Tell ExecTasksRequestHandler to run
                // Step 1
                result.Status = TaskBackupStatus.ScanningSourceFolder;
                AppSettings.Instance.UpdateTaskBackupResult(result);
                Thread.Sleep(100);
                // Step 2
                result.Status = TaskBackupStatus.ScanningDestinationFolder;
                AppSettings.Instance.UpdateTaskBackupResult(result);
                Thread.Sleep(100);
                // Step 3
                result.Status = TaskBackupStatus.Matching;
                AppSettings.Instance.UpdateTaskBackupResult(result);
                Thread.Sleep(100);
                // Step 4
                result.Status = TaskBackupStatus.Copying;
                AppSettings.Instance.UpdateTaskBackupResult(result);
            });
        }

The "marshalling" exception occurs at step 1.
The complete solution with the code of UpdateTaskBackupResult() can be found in the VS solution which you can find on my OneDrive public folder.
See AppSettings.zip.

For me, this looks like it is simply not allowed to write to local settings from within a background thread when closely before the thread is started there is a write from within the UI thread.

Additional observation after given a 2nd look to this:
Every time a write occurs to a local settings key the following error is shown in the VS output window:

onecoreuap\base\appmodel\statemanager\roamingrpc\stateroamingrpcclient.cpp(124)\Windows.Storage.ApplicationData.dll!7BF3D7B9: (caller: 7BF47C88) ReturnHr(2) tid(1f1c) 8007109A This operation is only valid in the context of an app container.

The error occurs both with simple string values and with composite data values. This seems to indicate that LocalSettings cannot be used in a WinUI Desktop app that is using the Windows App SDK. Not sure whether the "marshalling" exception is related to this, but I am not sure now whether LocalSettings can be used at all.

[JosHuybrighs]

@DrusTheAxe
I am not sure whether the "marshalling" issue is still relevant. I just installed Visual Studio 2022 and tried there the AppSetting.sln solution I posted earlier and it looks like the exception doesn't occur there. What is even more strange is that when I then tested this again with VS 2019 the error also suddenly doesn't appear anymore.
All strange for me but I assume something got updated during the installation of VS 2022.

The 'appcontainer' error/warning however is still there:

onecoreuap\base\appmodel\statemanager\roamingrpc\stateroamingrpcclient.cpp(124)\Windows.Storage.ApplicationData.dll!7BF3D7B9: (caller: 7BF47C88) ReturnHr(2) tid(1f1c) 8007109A This operation is only valid in the context of an app container.

This probably needs to be the subject of a new issue in order to close this one.

[DrusTheAxe]

I suspect that debugger message is a false one.

What version of Windows are you running on?

statemanager == ApplicationData. The 'roaming' tidbit is interesting as ApplicationData's roaming support was deprecated in 1909.

@alexlamtest is this a known issue?

[JosHuybrighs]

@alexlamtest
The issue about the marshalling exception can be closed. Doesn't appear anymore since installation of VS2022.
Remaining warning/errors moved to #1937