Skip to content
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.

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

AccessViolationException when running inside of a AppDomain #586

Closed
mmanela opened this issue Aug 27, 2017 · 42 comments
Closed

AccessViolationException when running inside of a AppDomain #586

mmanela opened this issue Aug 27, 2017 · 42 comments

Comments

@mmanela
Copy link

mmanela commented Aug 27, 2017

Note: This may be related to#383 but I am filing separately since it has its own unique repro

When I try to run very simple JS code from EdgeJS it throws a AccessViolationException when run inside of an appdomain (see stack below) 100% of the time.

This happens from my sample code below and from inside of an xunit unit test.

This code reproduces the issue:

[Serializable]
    class Program
    {
        static void Main(string[] args)
        {
            var p = new Program();
            if (args.Length > 0 && args[0] == "test")
            {
                p.Run();
            }
            else
            {
                p.RunInAppDomain();
            }
        }

        public void Run()
        {
            Test().Wait();
        }

        public void RunInAppDomain()
        {
            var assemblyFilename = typeof(Program).Assembly.Location;
            RunExeInAppDomain(assemblyFilename);
        }

        public static async Task Test()
        {
            var onMessage = (Func<object, Task<object>>)( (message) =>
            {
                Console.WriteLine($"Got this message: {message}");
                return Task.FromResult((object)"ok");
            });

            var func = Edge.Func(@"
            return function (params, callback) {

                params.onMessage('!!_!! This is cool', function(error, result) {});
                callback(null, 'Node.js welcomes ' + params.message);
            }
        ");
            
            Console.WriteLine(await func(new { message = "YES", onMessage = onMessage }));
        }

        static void RunExeInAppDomain(string assemblyFilename)
        {
            // Create an Application Domain:
            System.AppDomain newDomain = System.AppDomain.CreateDomain("NewApplicationDomain");

            // Load and execute an assembly:
            newDomain.ExecuteAssembly(assemblyFilename,new[] { "test" });

            // Unload the application domain:
            System.AppDomain.Unload(newDomain);
        }
    }
------ Run test started ------
[xUnit.net 00:00:00.2734579]   Starting:    EdgeTest
System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
   at _atexit_helper(IntPtr func, UInt32* __pexit_list_size, (fnptr)** __ponexitend_e, (fnptr)** __ponexitbegin_e)
   at _atexit_m(IntPtr func)
   at ClrFunc.Initialize(Local<v8::Function>* , Func`2 func)
   at ClrFunc.Initialize(FunctionCallbackInfo<v8::Value>* info)
   at initializeClrFunc(FunctionCallbackInfo<v8::Value>* info)
   at Nan.imp.?A0x8dda69af.FunctionCallbackWrapper(FunctionCallbackInfo<v8::Value>* info)
   at EdgeJs.Edge.NodeStartx86(Int32 argc, String[] argv)
   at EdgeJs.Edge.<>c__DisplayClass11_0.<Func>b__0()
   at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart()
   at _atexit_helper(IntPtr func, UInt32* __pexit_list_size, (fnptr)** __ponexitend_e, (fnptr)** __ponexitbegin_e)
   at _atexit_m(IntPtr func)
   at ClrFunc.Initialize(Local<v8::Function>* , Func`2 func)
   at ClrFunc.Initialize(FunctionCallbackInfo<v8::Value>* info)
   at initializeClrFunc(FunctionCallbackInfo<v8::Value>* info)
   at Nan.imp.?A0x8dda69af.FunctionCallbackWrapper(FunctionCallbackInfo<v8::Value>* info)
   at EdgeJs.Edge.NodeStartx86(Int32 argc, String[] argv)
   at EdgeJs.Edge.<>c__DisplayClass11_0.<Func>b__0()
   at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart()
@mmanela mmanela changed the title AccessViolationException when running inside of a XUnit test AccessViolationException when running inside of a AppDomain Aug 28, 2017
@mmanela
Copy link
Author

mmanela commented Aug 29, 2017

I turned on the EDGE_DEBUG environment variable and this is what it prints

Load edge native library from: C:\dev\EdgeVA_Repro\EdgeTest\bin\Debug\edge\x86\edge_nativeclr.node
edge::init
V8SynchronizationContext::Initialize
V8SynchronizationContext::Unref
CallbackHelper::Initialize
ClrFunc::Initialize MethodInfo wrapper
ClrFunc::Initialize Func<object,Task<object>> wrapper

Unhandled Exception: System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
   at _atexit_helper(IntPtr func, UInt32* __pexit_list_size, (fnptr)** __ponexitend_e, (fnptr)** __ponexitbegin_e)
   at _atexit_m(IntPtr func)
   at ClrFunc.Initialize(Local<v8::Function>* , Func`2 func)
   at ClrFunc.Initialize(FunctionCallbackInfo<v8::Value>* info)
   at initializeClrFunc(FunctionCallbackInfo<v8::Value>* info)
   at Nan.imp.?A0x8dda69af.FunctionCallbackWrapper(FunctionCallbackInfo<v8::Value>* info)
   at EdgeJs.Edge.NodeStartx86(Int32 argc, String[] argv)
   at EdgeJs.Edge.<>c__DisplayClass11_0.<Func>b__0()
   at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart()

@agracio
Copy link
Collaborator

agracio commented Sep 4, 2017

I have merged #573 to edge-js fork and published it to npm: https://www.npmjs.com/package/edge-js. Please let me know if it fixes the issue.

@mmanela
Copy link
Author

mmanela commented Sep 4, 2017

Thanks!, @agracio Can you also publish the nuget version also?

@agracio
Copy link
Collaborator

agracio commented Sep 4, 2017

Unfortunately I haven't gotten around to creating nuget package, but you can grab pre-compiled binaries from the release page https://github.com/agracio/edge-js/releases/tag/v8.2.2.
Direct link: https://github.com/agracio/edge-js/releases/download/v8.2.2/edge.js.zip

@mmanela
Copy link
Author

mmanela commented Sep 4, 2017

@agracio I am trying to validate with the zip you linked to but I am getting this error

>.\bin\Debug\EdgeTest.exe
module.js:598
  return process.dlopen(module, path._makeLong(filename));
                 ^

Error: %1 is not a valid Win32 application.
\\?\C:\dev\EdgeVA_Repro\EdgeTest\bin\Debug\edge\x86\edge_nativeclr.node
    at Object.Module._extensions..node (module.js:598:18)
    at Module.load (module.js:503:32)

@agracio
Copy link
Collaborator

agracio commented Sep 4, 2017

Not sure what it means, do note that it was built using NodeJs 8.2.1. Perhaps try using npm module directly without relying on zip package.

@mmanela
Copy link
Author

mmanela commented Sep 4, 2017

Hmm, got the same thing with NPM. Not sure if I am doing something wrong though.

@agracio
Copy link
Collaborator

agracio commented Sep 4, 2017

Library build is working fine in both Linux and Windows test environments. Travis: https://travis-ci.org/agracio/edge-js. Appveyor: https://ci.appveyor.com/project/agracio/edge-js.
What version of NodeJs are you using?

@mmanela
Copy link
Author

mmanela commented Sep 4, 2017

v8.4.0

@agracio
Copy link
Collaborator

agracio commented Sep 4, 2017

Just tried creating a test NodeJs project, ran npm install edge-js and executed a basic scipt

var edge = require('edge-js');

var helloWorld = edge.func(function () {/*
    async (input) => { 
        return ".NET Welcomes " + input.ToString(); 
    }
*/});

helloWorld('JavaScript', function (error, result) {
    if (error) throw error;
    console.log(result);
});

Runs without any problems. Haven't tried to run NodeJs from .NET app though.

@mmanela
Copy link
Author

mmanela commented Sep 9, 2017

Hmm, I wonder what the issue is. The version I consume directly from Nuget (8.2.1) works fine. Is there anything different that happens in the nuget packaging process?

@agracio
Copy link
Collaborator

agracio commented Sep 9, 2017

To my knowledge I use the same process to build the package. Or at least thats what I think :)
I use tools/build_double.bat 8.2.1 command to build it.

@mmanela
Copy link
Author

mmanela commented Sep 9, 2017

Could you try the nuget package and send it to me so I can try that? I tried myself but I am hitting lots of issues running build_double.

@agracio
Copy link
Collaborator

agracio commented Sep 9, 2017

Not sure what you mean by trying nuget package and sending it to you :)
Could you share an example project so I can try it out?

@mmanela
Copy link
Author

mmanela commented Sep 9, 2017

Here is my repro of the original issue.

And if you build and then run .\EdgeTest\bin\Debug\EdgeTest.exe you should see

Unhandled Exception: System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
   at _atexit_helper(IntPtr func, UInt32* __pexit_list_size, (fnptr)** __ponexitend_e, (fnptr)** __ponexitbegin_e)
   at _atexit_m(IntPtr func)
   at ClrFunc.Initialize(Local<v8::Function>* , Func`

I tried then replacing with your dll and got the invalid application issue.

@agracio
Copy link
Collaborator

agracio commented Sep 9, 2017

Yep, same thing happens on my PC too, something is wrong with the build process. Will try rebuilding.

@mmanela
Copy link
Author

mmanela commented Sep 9, 2017

Well, at least I am not crazy ;)

@agracio
Copy link
Collaborator

agracio commented Sep 10, 2017

No luck so far, tried rebuilding multiple times following these steps: https://github.com/agracio/edge-js#building-edgejs-nuget-package, but keep getting Error: %1 is not a valid Win32 application.
Have another idea in mind, but will take some time to test.

@mmanela
Copy link
Author

mmanela commented Sep 15, 2017

@agracio What is your other idea?

@agracio
Copy link
Collaborator

agracio commented Sep 15, 2017

Setting up a Windows 7 Azure VM with VS2015. Will try to build on it instead of my personal PC.
Not very optimistic to be honest since my PC successfully builds all the binaries in /lib/native/ folders, it is unlikely that different build setup would help.

@mmanela
Copy link
Author

mmanela commented Sep 15, 2017 via email

@agracio
Copy link
Collaborator

agracio commented Sep 16, 2017

Ok, so here is where i got to so far.
First about your VS project.
I have noticed that you running it in x86 and it throws AccessViolationException when running in AppDomain mode. When I tried switching it to x64 it produces no output when running in 'AppDomain' mode. Let me know if this is correct or it's something specific to my PC.

Now about the binary build.
After hours of looking for a way to correctly compile it I finally spotted an error in build_double.bat and made some progress. Unfortunately x86 still produces unusable binary, however x64 binary does work.
It correctly runs in AppDomain mode but throws exception when trying to unload it.

Got this message: !!_!! This is cool
Node.js welcomes YES

Unhandled Exception: System.CannotUnloadAppDomainException: Error while unloading appdomain. (Exception from HRESULT: 0x80131015)
   at System.AppDomain.Unload(AppDomain domain)
   at EdgeTest.Program.RunExeInAppDomain(String assemblyFilename) in C:\Users\anton\Dev\EdgeJsRepro\EdgeTest\Program.cs:line 65
   at EdgeTest.Program.RunInAppDomain() in C:\Users\anton\Dev\EdgeJsRepro\EdgeTest\Program.cs:line 34
   at EdgeTest.Program.Main(String[] args) in C:\Users\anton\Dev\EdgeJsRepro\EdgeTest\Program.cs:line 22

I will continue to work on x86 binary but in the meantime let me know your thoughts on the above,

@mmanela
Copy link
Author

mmanela commented Sep 16, 2017

Looking at docs it says that error can be caused by this:

If a thread does not abort, for example because it is executing unmanaged code, or because it is executing a finally block, then after a period of time a CannotUnloadAppDomainException is thrown in the thread that originally called Unload.

But not sure how that could happen since the code waits on the EdgeJs task before continuing.

   public void Run()
        {
            Test().Wait();
        }

@agracio
Copy link
Collaborator

agracio commented Sep 16, 2017

So that actually explains the problem, unmanaged code is still executing. I noticed that in my Node.js -> .NET apps edge unmanaged code continues to run after the call is completed. You could handle exception or manage creating/unloading app domain elsewhere.

Whats important here is that #573 appears to solve AccessViolationException problem. As soon as I figure out how to compile x86 correctly I will post binaries.

@mmanela
Copy link
Author

mmanela commented Sep 17, 2017

Interesting, how can c# know when the native work is done?

@agracio
Copy link
Collaborator

agracio commented Sep 18, 2017

.NET cannot really have control over unmanaged resources. Unfortunately I work with large .NET applications that use unmanaged components and when you really need to terminate native component and free all the resources it is holding the only sure way it to terminate and restart .NET process itself.
I think you are misunderstanding the term 'native work is done' in this case :). Edge.js native code might have completed your request but that does not mean the unmanaged code itself has terminated. It could be still finishing internal processes and in that case you could try unloading AppDomain using a 'timeout' and handle the exception if it does not unload after given time. Something along these lines: https://stackoverflow.com/questions/4432141/how-to-properly-unload-an-appdomain-using-c.

@mmanela
Copy link
Author

mmanela commented Sep 22, 2017

Cool sounds fine. Any idea what the x64 issue remaining is?

@agracio
Copy link
Collaborator

agracio commented Sep 22, 2017

The issue is with x86 build, I got x64 working. Your solution is set to 32 bit and was using x86 binary.

@mmanela
Copy link
Author

mmanela commented Sep 22, 2017

Ok, so there is not issue in the build? Then the PR is all ready to merge? Do we need to merge your build fixes into it?

@agracio
Copy link
Collaborator

agracio commented Sep 23, 2017

This is not the only PR that is ready to be merged ;) The entire reason I maintain a fork is the fixes it has that have not been merged to edge. Some are PRs from this repo and some don't have PRs as I did not bother creating any more seeing as nothing is merged. This #566 PR for example is essential to my production apps.
As for nuget build process I am guessing @tjanczuk has his own way to build binaries, so my fix is not required.

@mmanela
Copy link
Author

mmanela commented Sep 24, 2017

Ok, can you give me the updated zip of the dlls so I can at least unblock my project.

@agracio
Copy link
Collaborator

agracio commented Sep 24, 2017

Latest binaries are attached to v.8.2.5 release. Only x64 binary works, so make sure your projects are not set to build at 32 bit like the test project you shared.

@mmanela
Copy link
Author

mmanela commented Sep 24, 2017 via email

@agracio
Copy link
Collaborator

agracio commented Sep 24, 2017

Not really, something is wrong with the build process but I cannot figure it out. See if you have better luck. Start with this fix to build_double.bat and see if you can figure the rest out.

@mmanela
Copy link
Author

mmanela commented Sep 24, 2017

Hmm, Ill try. Last time when I ran build_double, it failed on some other error. But will try again and see.

@mmanela
Copy link
Author

mmanela commented Sep 24, 2017

Yea, this is what I get trying to build

creating config.gypi
creating config.mk
Traceback (most recent call last):
  File "configure", line 1415, in <module>
    run_gyp(gyp_args)
  File "tools\gyp_node.py", line 53, in run_gyp
    rc = gyp.main(args)
  File "tools\gyp\pylib\gyp\__init__.py", line 538, in main
    return gyp_main(args)
  File "tools\gyp\pylib\gyp\__init__.py", line 523, in gyp_main
    generator.GenerateOutput(flat_list, targets, data, params)
  File "tools\gyp\pylib\gyp\generator\msvs.py", line 2032, in GenerateOutput
    generator_flags))
  File "tools\gyp\pylib\gyp\generator\msvs.py", line 953, in _GenerateProject
    return _GenerateMSBuildProject(project, options, version, generator_flags)
  File "tools\gyp\pylib\gyp\generator\msvs.py", line 3378, in _GenerateMSBuildProject
    project_file_name)
  File "tools\gyp\pylib\gyp\generator\msvs.py", line 2714, in _GetMSBuildGlobalProperties
    _ConfigWindowsTargetPlatformVersion(configuration, version))
  File "tools\gyp\pylib\gyp\generator\msvs.py", line 311, in _ConfigWindowsTargetPlatformVersion
    return names[0]
IndexError: list index out of range
Failed to create vc project files.
Cannot build node.dll for 8.2.1-x86

@agracio
Copy link
Collaborator

agracio commented Sep 24, 2017

Looks like error during gyp config rather than compile error. Are you able to build just edge by running something like build.bat release 8.2.1?

@mmanela
Copy link
Author

mmanela commented Sep 24, 2017

That seems to work ok

  Generating code
  Finished generating code
  edge_coreclr.vcxproj -> C:\dev\edge\build\Release\\edge_coreclr.node
  edge_coreclr.vcxproj -> C:\dev\edge\build\Release\edge_coreclr.pdb (Full PDB)
gyp info ok
C:\dev\edge\tools\\..\lib\native\win32\x64\8.2.1
.\build\release\edge_coreclr.node
.\build\release\edge_nativeclr.node
        2 file(s) copied.
C:\dev\edge\tools\\..\lib\native\win32\x64\8.2.1\..\concrt140.dll
C:\dev\edge\tools\\..\lib\native\win32\x64\8.2.1\..\msvcp140.dll
C:\dev\edge\tools\\..\lib\native\win32\x64\8.2.1\..\vccorlib140.dll
C:\dev\edge\tools\\..\lib\native\win32\x64\8.2.1\..\vcruntime140.dll
        4 file(s) copied.
Success building edge.node release for node.js x64 v8.2.1

@Michael-Tajmajer-Emrsn
Copy link

The issue with the x86 build is due to build_double.bat:

"%SELF%\build\repl.exe" ./build/edge_nativeclr.vcxproj "%USERPROFILE%.node-gyp%1%2\node.lib" "%SELF%build\node-%1-%2\node.lib"

%2 is x86 -- but it needs to be ia32 for %USERPROFILE%.node-gyp%1%2\node.lib
and it needs to be x86 for "%SELF%build\node-%1-%2\node.lib"

I changed it to:
"%SELF%\build\repl.exe" ./build/edge_nativeclr.vcxproj "%USERPROFILE%.node-gyp%1%2\node.lib" "%SELF%build\node-%1-%3\node.lib"

and changed the calling lines to:
call :build_edge %1 ia32 x86
call :build_edge %1 x64 x64

After this, the win32 version ran fine.

As an aside, has anybody managed to get the 8.9.0 (LTS) version of node building?

I end up with a bunch of "cannot convert argument 1 from 'nullptr' to 'nullptr &&'" errors...

@mmanela
Copy link
Author

mmanela commented Nov 7, 2017

@Michael-Tajmajer-Emrsn That is awesome. @agracio could you take his change and build updated binaries for me?

@agracio
Copy link
Collaborator

agracio commented Nov 7, 2017

@Michael-Tajmajer-Emrsn thanks for this update, I've merged your PR and both x86 and x64 binaries are now working fine. Latest edge-js release includes both v8.9.0 LTS and new v9.0.0.
You need to set msvs_version to 2017 in order to build Node.js 8.3.+ https://github.com/agracio/edge-js/blob/master/tools/build.bat#L57.
@mmanela both x86 and x64 binaries are part of 9.0.0 release https://github.com/agracio/edge-js/releases/tag/v9.0.0.

@agracio
Copy link
Collaborator

agracio commented Dec 23, 2017

Binaries are now also published to NuGet

@agracio agracio closed this as completed Jun 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants