ExcelAsyncUtil.Observe under the same lock causes dealock for IsThreadSafe=true function

[emphasis87]

To test it, run the attached solution, in A column run =MyData(), in B column run =MyFunction(A1) and expand the formula, which hangs the document.

Or is this the expected behavior?

ObserveExample.zip

[govert]

I can reproduce the issue you see with your project. I'm not sure what my expectations of this code would be, and it will take some debugging to see why we get the deadlock.

[govert]

Can you describe a bit why you want this pattern to work? The high-level lock is kind of in contradiction to the IsThreadSafe=true option. That does not explain the deadlock, of course.

Is there a way to rewrite your function as

        [ExcelFunction(IsThreadSafe = true)]
        public static IObservable<object> MyFunction(string value) { ... }

With ExcelDna.AddIn version 1.9.0 this should be wrapped in an ExcelAsyncUtil.Observe call and registered automatically.

[emphasis87]

I am accessing a concurrent dictionary to cache the observable based on the parameters of the function. An external service then updates the observable as updates come. We also support VBA in the same function, so I can not directly transform it to IObservable return type, since the client wants the latest result, not the observable itself there.

I wanted to prevent a possibility of stale observable being registered in ExcelAsyncUtil.Observe. The observable I am using tracks the number of subscribers, on the first one it does initialization work, on removing the last one it cleans itself from the dictionary, both under the lock.

The lock is segmented based on the key hash, so not the same object is used for locking all the keys.

I can solve the issue by a dummy subscribe inside the lock and disposing it after the ExcelAsyncUtil.Observe call is finished outside the lock.

Since I was using multiple objects for locking it sometimes worked, sometimes bugged out. After figuring out that ExcelAsyncUtil.Observe is causing the issue, I looked into its code, but could not find what is causing the deadlock. So I simplified the case to the simplest I could reproduce.

[govert]

If you are making a function that streams results into a cell, then a good way to think about it is that you are making a function with this signature:

        [ExcelFunction]
        public static IObservable<object> MyFunction(string value) { ... }

Under Excel-DNA v 1.9.0-beta1 this is automatically wrapped and registered, but is equivalent to something you could write in pre-1.9.0 versions like this

        [ExcelFunction]
        public static object MyFunction(string value) 
        {
             return ExcelAsyncUtil.Observe(nameof(MyFunction), new object[] { value }, () => MyFunctionImpl(value));
        }

Then Excel-DNA will take care of:

• Making sure there is only one IObservable created per set of input arguments. Your function with the above signature will only be called once per set of input arguments. You must make an IObservable using those arguments, which will (after Subscribe is called, via the IObserver) stream back the different values to be returned to the cell. Excel-DNA and Excel's RTD mechanism takes care of triggering the cell update and returning the latest stream values.
• There will be exactly one Subscriber to this IObservable.
• IDisposable.Dispose will be called on the IObserver returned from the IObservable.Subscribe when the formula is deleted from the cell, or an input argument is changed.

I suggest you consider refactoring the bits you want to expose into the above signature, and using the IDisposable.Dispose for the cleanup.

I'm not sure how you'd use this from VBA, but getting it right from the sheet seems like a good start.

[emphasis87]

The client calls it using Application.Run.

The caching scheme is a bit more complicated, for some functions the caller is part of the cache key, for others it isn't and the observable is shared. Also the arguments are parsed and checked if they are functionally equivalent. Fun stuff.

I will look into the 1.9.0 possibilities once it comes out. I had some issues w 1.8.0 last week when updating VS 2022 lead to not being able to build, due to reference to Microsoft.Bcl. I checked the latest beta and there are some errors in the build output, haven't had the time to check more.

[govert]

If you want the caller part of the RTD topic key then you can do it like this:

        [ExcelFunction]
        public static object MyFunction(string value) 
        {
             var caller = XlCall.Excel(XlCall.xlfCaller);
             return ExcelAsyncUtil.Observe(nameof(MyFunction), new object[] { value, caller }, () => MyFunctionImpl(value));
        }

Without the caller, only the function name and arguments array are used to identify the topic.

If you can use Excel-DNA to track the right IObservable that match the call, then the code might be simpler and more likely to work right.

[emphasis87]

I already have this covered, the code is working, no worries. The only issue I came across was the ExcelAsyncUtil.Observe under the lock, which I extracted outside the critical section. I reported it because the expectation from my side was that it was threadsafe and from the ExcelDna code I could not figure out where the deadlock is happening. If you solve it, you will save me one allocation of observable subscription and its cleanup, but it is not critical.