8

I'm about to start a migration project at work for a legacy system written in VBScript. It has an interesting structure in that much of it was segregated by writing various components as "WSC" files, which are effectively a way of exposing VBScript code in a COM-like manner. The boundary interface from the "core" to these components is fairly tight and well known so I was hoping that I would be able to tackle writing a new core and reuse the WSCs, postponing their rewriting.

It's possible to load a WSC by adding a reference to "Microsoft.VisualBasic" and calling

var component = (dynamic)Microsoft.VisualBasic.Interaction.GetObject("script:" + controlFilename, null);

where "controlFilename" is the full file path. GetObject returns a reference of type "System.__ComObject" but the properties and methods can be accessed using .net's "dynamic" type.

This appeared to work fine initially, but I've encountered problems when quite a specific set of circumstances are combined - my worry is that this may happen in other cases or, even worse, that bad things are happening much of the time and being masked, just waiting to blow up when I least expect it.

The raised exception is of type "System.ExecutionEngineException", which sounds particularly scary (and vague)!

I've cobbled together what I believe to be the minimum reproduce case and was hoping that someone could cast a little light on what the problem could be. I've also identified some tweaks that can be made that seem to prevent it, though I can't explain why.

  1. Create a new empty "ASP.NET Web Application" called "WSCErrorExample" (I've done this in VS 2013 / .net 4.5 and VS 2010 / .net 4.0, it makes no difference)

  2. Add a reference to "Microsoft.VisualBasic" to the project

  3. Add a new "Web Form" called "Default.aspx" and paste the following over the top of "Default.aspx.cs"

    using System;
    using System.IO;
    using System.Reflection;
    using System.Runtime.InteropServices;
    using Microsoft.VisualBasic;
    
    namespace WSCErrorExample
    {
        public partial class Default : System.Web.UI.Page
        {
            protected void Page_Load(object sender, EventArgs e)
            {
                var currentFolder = GetCurrentDirectory();
                var logFile = new FileInfo(Path.Combine(currentFolder, "Log.txt"));
                Action<string> logger = message =>
                {
                    // The try..catch is to avoid IO exceptions when reproducing by requesting the page many times
                    try { File.AppendAllText(logFile.FullName, message + Environment.NewLine); }
                    catch { }
                };
    
                var controlFilename = Path.Combine(currentFolder, "TestComponent.wsc");
                var control = (dynamic)Interaction.GetObject("script:" + controlFilename, null);
    
                logger("About to call Go");
                control.Go(new DataProvider(logger));
                logger("Completed");
            }
            private static string GetCurrentDirectory()
            {
                // This is a way to get the working path that works within ASP.Net web projects as well as Console apps
                var path = Path.GetDirectoryName(Assembly.GetExecutingAssembly().GetName().CodeBase);
                if (path.StartsWith(@"file:\", StringComparison.InvariantCultureIgnoreCase))
                    path = path.Substring(6);
                return path;
            }
    
            [ComVisible(true)]
            public class DataProvider
            {
                private readonly Action<string> _logger;
                public DataProvider(Action<string> logger)
                {
                    _logger = logger;
                }
    
                public DataContainer GetDataContainer()
                {
                    return new DataContainer();
                }
    
                public void Log(string content)
                {
                    _logger(content);
                }
            }
    
            [ComVisible(true)]
            public class DataContainer
            {
                public object this[string fieldName]
                {
                    get { return "Item:" + fieldName; }
                }
            }
        }
    }
    
  4. Add a new "Text File" called "TestComponent.wsc", open its properties window and change "Copy to Output Directory" to "Copy if newer" then paste the following in as its content

    <?xml version="1.0" ?>
    <?component error="false" debug="false" ?>
    <package>
        <component id="TestComponent">
            <registration progid="TestComponent" description="TestComponent" version="1" />
            <public>
                <method name="Go" />
            </public>
            <script language="VBScript">
                <![CDATA[
                    Function Go(objDataProvider)
                        Dim objDataContainer: Set objDataContainer = objDataProvider.GetDataContainer()
                        If IsEmpty(objDataContainer) Then
                            mDataProvider.Log "No data provided"
                        End If
                    End Function
            ]]>
            </script>
        </component>
    </package>
    

Running this once should cause no apparent issue, the "Log.txt" file will be written to the "bin" folder. Refreshing the page, however, normally results in an exception

Managed Debugging Assistant 'FatalExecutionEngineError' has detected a problem in 'C:\Program Files (x86)\IIS Express\iisexpress.exe'.

Additional information: The runtime has encountered a fatal error. The address of the error was at 0x733c3512, on thread 0x1e10. The error code is 0xc0000005. This error may be a bug in the CLR or in the unsafe or non-verifiable portions of user code. Common sources of this bug include user marshaling errors for COM-> interop or PInvoke, which may corrupt the stack.

Occasionally, the second request does not result in this exception, but holding down F5 in the browser window for a couple of seconds will ensure that it rears its ugly head. The exception, so far as I can tell, happens at the "If IsEmpty" check (other versions of this reproduce case had more logging calls, which pointed to that line being the source of the problem).

I've tried various things to try to get to the bottom of this, I've tried to recreate in a console app and the problem does not occur, even if I spin up hundreds of threads and get them to process the work above. I've tried an ASP.Net MVC web application, rather than using a Web Form and the same issue DOES occur. I've tried changing the apartment state from the default MTA to STA (I was clutching at straws a bit at that point!) and it made no change to the behavour. I've tried building a web project that uses Microsoft's OWIN implementation and the issue occurs in that scenario as well.

Two interesting things that I have noticed - if the "DataContainer" class does not have an indexed property (or a default method / property, decorated with a [DispId(0)] attribute - not illustrated in this example) then the error does not occur. If the "logger" closure does not contain a "FileInfo" reference (if a string "logFilePath" was maintained, rather than the FileInfo instance "logFile") then the error does not occur. I suppose it sounds like one approach would be to avoid doing these things! But I would be concerned that there could be other ways to trigger this scenario that I don't currently know about and trying to enforce the rule of not-doing-these-things could get complicated as the code base grows, I can imagine this error creeping back in without it being immediately obvious why.

On one run (through Katana), I got additional call stack information:

This thread is stopped with only external code frames on the call stack. External code frames are typically from framework code but can also include other optimized modules which are loaded in the target process.

Call stack with external code

mscorlib.dll!System.Variant.Variant(object obj) mscorlib.dll!System.OleAutBinder.ChangeType(object value, System.Type type, System.Globalization.CultureInfo cultureInfo) mscorlib.dll!System.RuntimeType.TryChangeType(object value, System.Reflection.Binder binder, System.Globalization.CultureInfo culture, bool needsSpecialCast) mscorlib.dll!System.RuntimeType.CheckValue(object value, System.Reflection.Binder binder, System.Globalization.CultureInfo culture, System.Reflection.BindingFlags invokeAttr) mscorlib.dll!System.Reflection.MethodBase.CheckArguments(object[] parameters, System.Reflection.Binder binder, System.Reflection.BindingFlags invokeAttr, System.Globalization.CultureInfo culture, System.Signature sig) [Native to Managed Transition]

One final note: if I create a wrapper for the "DataProvider" class, using IReflect and map the calls over IDispatch on to calls to the underlying "DataProvider" instance then the issue goes away. But again, deciding that this is somehow the answer seems dangerous to me - if I have to be meticulous about ensuring that any reference passed to the components has such a wrapper then errors could creep in that could be difficult to track down. What if a reference that IS encased in an IReflect-implementing wrapper returns a reference from a method or property call that isn't wrapped up in the same way? I suppose the wrapper could try to do something like ensuring it only returns "safe" reference (ie. those without indexed properties or DispId=0 methods or properties) without wrapping them in a further IReflect wrapper.. but it all seems a bit hacky.

I really have no clue where next to go with this problem, does anyone have any idea?

John Saunders
  • 160,644
  • 26
  • 247
  • 397
Dan Roberts
  • 2,244
  • 16
  • 31
  • I would think that you might need to ensure that the WSC code is only ever accessed from a single thread at a time. You might need to put `lock` statements around the use of that code. I doubt it was meant to run in a multi-threaded environment like that of ASP.NET. – John Saunders Jul 16 '14 at 00:12
  • I've tried putting in a static lock object to Default.aspx.cs and wrapping the work in a lock using that, but still no joy. I would have been surprised if it had made a difference since the error occurs when the first request has completely terminated and a second (non-concurrent) request comes in. But it was worth a try, so thanks! :) – Dan Roberts Jul 16 '14 at 09:25

1 Answers1

2

My guess is, the error you're seeing is caused by the fact that WSC script components are COM STA objects by their nature. They're implemented by the underlying VBScript Active Scripting Engine, which itself is an STA COM object. As such, they require an STA thread to be created and accessed on, and such thread should remain the same for the lifetime of any particular WSC object (the object requires thread affinity).

ASP.NET threads are not STA. They're ThreadPool threads, and they implicitly become COM MTA threads when you start using COM objects on them (for differences between STA and MTA, refer to INFO: Descriptions and Workings of OLE Threading Models). COM then creates a separate implicit STA apartment for your WSC objects and marshal calls there from your ASP.NET request thread. The whole thing may or may not go well in the ASP.NET environment.

Ideally, you should get rid of WSC script components and replace them with .NET assemblies. If that's not feasible short-term, I'd recommend that you run your own explicitly controlled STA thread(s) to host the WSC components. The following may help:

Updated, why not give this a try? Your code would look like this:

// create a global instance of ThreadAffinityTaskScheduler - per web app
public static class GlobalState 
{
    public static ThreadAffinityTaskScheduler TaScheduler { get; private set; }

    public static GlobalState() 
    {
        GlobalState.TaScheduler = new ThreadAffinityTaskScheduler(
            numberOfThreads: 10,
            staThreads: true, 
            waitHelper: WaitHelpers.WaitWithMessageLoop);
    }
}

// ... inside Page_Load

GlobalState.TaScheduler.Run(() => 
{
    var control = (dynamic)Interaction.GetObject("script:" + controlFilename, null);

    logger("About to call Go");
    control.Go(new DataProvider(logger));
    logger("Completed");

}, CancellationToken.None).Wait();

If that works, you can somewhat improve the web app's scalabilty by using PageAsyncTask and async/await instead of the blocking Wait().

Community
  • 1
  • 1
noseratio
  • 59,932
  • 34
  • 208
  • 486
  • 2
    Thanks for your response! I had tried adding AspCompat="true" to the Page declaration which, as I understand it, tells the Web Form to run in a thread in STA mode. Changing the "About to call Go" logger call to logger("About to call Go - Hosting Thread has ApartmentState: " + System.Threading.Thread.CurrentThread.GetApartmentState()) appears to confirm that this has worked, and yet the problem still occurs. Is that what you meant? Rewriting the old code into .net is definitely the ideal (and the longer-term plan), but I think it might be biting off too much to try to do it all in one go. – Dan Roberts Jul 15 '14 at 15:45
  • @DanDanDan, `AspCompat="true"` should indeed have mitigated the MTA issues, but apparently it didn't. I'll play with your code when time allows. Have you tried executing it outside ASP.NET, with unit tests, or just a simple console app (on an STA thread)? – noseratio Jul 16 '14 at 00:20
  • I did try it with a console app and thought that I'd been unable to reproduce the problem. However, I've started from scratch again and got an app that spins up hundreds of threads and tries to perform the work. If I don't set the apartment state on the threads to STA then _most times_ it runs, the app crashes (without helpful exception information). With apartment state set to STA it _appears_ to work most times: https://gist.github.com/anonymous/80a2fa7f8589ee8cf92e. I'm still clueless as to why it fails in ASP.Net with the compat (STA-enabling) mode though. – Dan Roberts Jul 16 '14 at 09:10
  • @DanDanDan, check my update. I'd be interested to know if it's still breaking that way. – noseratio Jul 16 '14 at 10:00
  • 1
    this looks extremely promising! On initial inspection I can't seem to reproduce the error. I'm just going to play around with it a bit more and then accept your answer if it still looks good. This is really good news, thank you very much! :) Guess I'm going to have to dive in and work out what it all means (starting with your previous answers you've linked). Very minor point: in your example code, the "GlobalState" constructor needed brackets as the "public" accessor removed, then everything compiled fine. – Dan Roberts Jul 16 '14 at 13:19
  • @DanDanDan, glad if it's helping. I fixed the typo in the sample - I typed it out of head here, but I did test the GitGub code. – noseratio Jul 16 '14 at 13:29
  • 1
    I've tested this more thoroughly and I can't break it in the variety of circumstances I've tried it in - so I'm considering this a success! Thanks again for your help, I'm definitely going to read all of the information that you've linked to to try to get understand the solution. Answer very happily accepted! – Dan Roberts Jul 16 '14 at 14:35
  • @DanDanDan, just make sure you maintain thread affinity if you use the same COM objects across multiple `TaScheduler.Run` calls. Here's a related [answer](http://stackoverflow.com/a/24781995/1768303) illustrating what I mean. – noseratio Jul 16 '14 at 14:59
  • I'm still reading through all of your information - not just that answer but other answers and articles that you link to, so apologies if this seems like a simple question. Should I conclude, if I want to make multiple calls to the same COM component, at various different times (so not multiple calls to methods on the component within a single Run call, thought it would be within a single page request), that "numberOfThreads" should always be one? There's no way to hand in and out a "flag" to indicate what thread the component used last time and so needs to again on the next call? – Dan Roberts Jul 16 '14 at 16:42
  • @DanDanDan, this is a good question and maybe is something I can improve the `ThreadAffinityTaskScheduler` upon. Yes, currently it so. However, it's possible to add an API to return a random `ThreadWithAffinityContext` (from the pool maintained by `ThreadAffinityTaskScheduler`), so it can be used across multiple `Run` calls or even multiple requests. For the latter, it needs to be registered with ASP.NET, via `HostingEnvironment.QueueBackgroundWorkItem` or `IRegisteredObject`, [more details](http://blog.stephencleary.com/2014/06/fire-and-forget-on-asp-net.html). – noseratio Jul 17 '14 at 00:38
  • I would be very interested in seeing anything you do with this! I'm trying to do something similar, and I think I'm getting my head around what everything is doing and why - but it's still going to be some time until I would declare myself fully competent on the subject! – Dan Roberts Jul 17 '14 at 12:21