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

JX_DefineMainFile not functioning in embedded Windows app #608

Closed
dustmop opened this issue Oct 15, 2015 · 19 comments · Fixed by #837
Closed

JX_DefineMainFile not functioning in embedded Windows app #608

dustmop opened this issue Oct 15, 2015 · 19 comments · Fixed by #837

Comments

@dustmop
Copy link
Contributor

dustmop commented Oct 15, 2015

Have embedded jxcore into a large, pre-existing Win32 application, by compiling with --shared_library and including the jx.dll binary in my solution. Though JX_Evaluate and JX_DefineExtension can correctly run js and hooks, the contents of JX_DefineMainFile are never executed.

Making a minimal command-line app that calls jxcore in the same way works perfectly fine, including the ...MainFile being run, so I'm not sure where the difference lies. Tracing the execution of JX_StartEngine in a debugger, I see a callstack like this:

engine->Start()
node::Load
JS_METHOD_CALL
i::Execution::Call
Invoke
CALL_GENERATED_CODE
(the debugger doesn't correctly step into this, so it's as far as I got)

So it looks the engine correctly sees that main has a function object defined, but doesn't execute it properly.

Any pointers on how to dig into this more deeply, diagnose the problem, or otherwise figure out what I'm doing wrong?

@obastemur
Copy link
Member

@dustmop need more info. Can you copy-paste the codes around JX_DefineMainFile ?

@dustmop
Copy link
Contributor Author

dustmop commented Oct 16, 2015

@obastemur

Embedded in the large GUI-based app:

void nullCallback(JXResult *results, int argc) {
  // do nothing
}

void showMsg(JXResult *results, int argc) {
  static int g_count = 0;
  JXValue* val = &results[0];
  std::string str = JX_GetString(val);
  printf("count %d msg %s\n", g_count, str.c_str());
  g_count += 1;
}

int JsEngineMain(int argc, char **args) {
  JX_Initialize(args[0], nullCallback);
  JX_InitializeNewEngine();

  char *contents = "process.natives.showMsg('a');";
  JX_DefineMainFile(contents);

  JX_DefineExtension("showMsg", showMsg);
  JX_StartEngine();
  while (JX_LoopOnce());

  JXValue ret_val;
  JX_Evaluate("process.natives.showMsg('b');", "eval", &ret_val);
  JX_Free(&ret_val);
  while (JX_LoopOnce());
  JX_StopEngine();
  return 0;
}

output is:
count 0 msg b

The command-line minimal version in its entirety is this with s/JsEngineMain/main/ and it's output is:
count 0 msg a
count 1 msg b

I don't know if the GUI app is threaded or not, could that be a problem? Does the linked subsystem matter? Even advice on how to debug this on my own would be appreciated.

@obastemur
Copy link
Member

I don't know if the GUI app is threaded or not, could that be a problem? Does the linked subsystem matter?

No. It shouldn't matter.

Do you have a separate main.js on the file system ?

@dustmop
Copy link
Contributor Author

dustmop commented Oct 16, 2015

Nope, no other main.js file.

DefineMainFile uses some sort of virtual file system, right? I was trying to figure out how it works, and saw it calls engine->MemoryMap at some point, but that seems to be optimized away in both the GUI app and command-line app.

@dustmop
Copy link
Contributor Author

dustmop commented Oct 21, 2015

I found that the implementation of DefineMainFile uses the MemoryMap object to store source files, which is later used by node.cc:Load when MainSource reads and executes a giant js blob called jxcore::node_native. I traced the calls that jxcore::node_native is making to MemoryMap.SourceRead in both apps, the working command-line app and broken GUI app. Both start like this:

MemoryWrap.SourceRead("config")
MemoryWrap.SourceRead("events")
MemoryWrap.SourceRead("buffer")
MemoryWrap.SourceRead("assert")
MemoryWrap.SourceRead("util")
MemoryWrap.SourceRead("stream")
MemoryWrap.SourceRead("_stream_readable")
MemoryWrap.SourceRead("_stream_writable")
MemoryWrap.SourceRead("_stream_duplex")
MemoryWrap.SourceRead("_stream_transform")
MemoryWrap.SourceRead("_stream_passthrough")
MemoryWrap.SourceRead("console")

then the command-line app keeps going with

MemoryWrap.SourceRead("tty")
MemoryWrap.SourceRead("net")
MemoryWrap.SourceRead("_jx_timers")
MemoryWrap.SourceRead("timers")
.....(etc etc)

whereas the GUI app stops at "console". You said the subsystem (windows vs console) shouldn't matter, but there's certainly some unhandled error happening related to the console js module. I think the application might be rebinding stdout somewhere? Perhaps the relevant js exception is being silently swallowed, is there some better way to get errors out of the executed js? I'll dig in some more when I have some time in a few days.

@dustmop
Copy link
Contributor Author

dustmop commented Nov 4, 2015

Ah, using freopen I see that stderr contains this:


Starting JXcore engine

node.js:1001
      throw new Error('Implement me. Unknown stream file type!');
            ^
Error: Implement me. Unknown stream file type!
    at createWritableStdioStream (node.js:1003:13)
    at process.stdout (node.js:1051:18)
    at console.js:190:37
    at NativeModule.compile (node.js:1371:5)
    at Function.NativeModule.require (node.js:1338:18)
    at startup (node.js:211:27)
    at node.js:1453:3
JXcore engine is started
Destroying JXcore engine
JXcore engine is destroyed

tty_wrap.guessHandleType is returning the string 'UNKNOWN'.

@obastemur
Copy link
Member

Seeing this for the first time. What are the repro steps ?

@dustmop
Copy link
Contributor Author

dustmop commented Nov 4, 2015

Let me try and come up with a minimal repo example and get back to you.

@dustmop
Copy link
Contributor Author

dustmop commented Nov 10, 2015

I have tracked down the root cause and have a repro. Turns out jxcore does not in fact work with Win32 gui applications, specifically those using subsystem:windows (as opposed to subsystem:console). Subsystem windows doesn't have stdout and stderr, similar to Android and WinRT, and should use fake streams.

Repro is here: https://github.com/dustmop/sample_win32

The fix is to detect whether the windows host is console or gui, perhaps by calling GetFileType(1), seeing if the result is FILE_TYPE_UNKNOWN, and setting a flag on process called "isWinGui". Then modify isSTD in src/node.js to also check for "process.platform == "windows" && process.isWinGui"

Also, the name "isSTD" seems to imply the opposite, that the platform has stdout. Perhaps change this to "isHeadless"?

I can put together a PR if you think this sounds correct.

@obastemur
Copy link
Member

Sounds okay. I would prefer the recent isWinRT approach that dumps the console.log and console.error into attached debugger. So we need

  • a compile time option that will set a flag
  • using that flag, rest of the framework will follow WinRT (console etc) implementation

If sounds reasonable and you can PR this, we would really appreciate to have this on 0.3.1.0!

@obastemur
Copy link
Member

a compile time option that will set a flag

So we will be creating a separate DLL for WinGUI apps. (--win-gui) -> node_for_win_gui -> WIN_APP_HAS_GUI -> isWinGUI

@dustmop
Copy link
Contributor Author

dustmop commented Nov 10, 2015

What's the benefit for having a compile-time flag? It would make a developer doing embedding have to be aware of the two different Windows flavors of jxcore, and have to choose the correct one when embedding into their host application. Furthermore, I would guess on Windows that the more common case by far would be embedding into a GUI app rather than console. Seems more error prone, compared to doing run-time detection.

I'm not familiar with WinRT, so I'm not sure what attached debugger refers to, but for standard Windows GUI apps there's no meaningful concept for any sort of console / stdout / stderr.

Aside from clearing that up, I believe I can get the PR done tomorrow, it seems fairly simple.

@obastemur
Copy link
Member

console stderr etc. are very useful when someone debugs that GUI app. (i.e. you can see the messages on MSVS Debug window)

Compile time flag prepares the entire solution (including native and some of the dependencies) to put the messages to the right window. Node is designed for console hence we need to redirect all those messages to somewhere meaningful. JXcore internally has this switch for both Android (logcat) and WinRT( MSVS Debugger Window) . I believe WinGUI can share the WinRT approach here.

I do agree on offering two sets of binary option is more confusing than a single one but much easier to go with current implementation and more node.js compatible. (in terms of usage and result)

IMHO, a developer doesn't really need those two libraries on a single project. You have either a GUI or a console app (most of the times).

Aside from clearing that up, I believe I can get the PR done tomorrow, it seems fairly simple.

sounds great!

@dustmop
Copy link
Contributor Author

dustmop commented Nov 11, 2015

I'll look into reusing the MSVS Debugger hook, but I'm not convinced this suggests a compile-time flag instead of run-time detection.

A flag would make it more difficult for another rando like me, who comes to jxcore blind, wanting to embed js into their Windows app. First, they try out the example code in a bare-bones console application, and it works fine, then they move that exact code to a GUI app and it breaks in a VERY subtle way (JS_Evaluate works ok, but main doesn't work and there's no error message anywhere). Main won't work in the GUI app until they recompile the jx.dll with a compile-tile flag, and then that dll won't work in the example code. So they have two dll's that are incompatible with each other and they need to juggle the right version into the appropriate app. Whereas with runtime detection this problem just goes away.

Maybe that's too difficult to implement, and I'll see that once I start coding it. First I'd like to give it a try, and we can discuss more in the PR.

@dustmop
Copy link
Contributor Author

dustmop commented Nov 11, 2015

Whelp, I broke my dev environment with a Windows Update, so this might take a bit longer. When is 0.3.1.0 scheduled to happen?

@dustmop
Copy link
Contributor Author

dustmop commented Nov 11, 2015

No wait, nevermind, my environment is fine. The build was broken by a3fc681, for Visual C++ 2010 Express. I'll look into fixing that first. The error is:

C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Microsoft.Cpp.InvalidPlatform.Targets(23,7): error MSB8007: The Platform for project 'cares.vcxproj' is invalid.  Platform='x64'. You may be seeing this message because you are trying to build a project without a solution file, and have specified a non-default Platform that doesn't exist for this project. [C:\path\to\jxcore\deps\cares\cares.vcxproj]
...repeats 17 times...

@dustmop
Copy link
Contributor Author

dustmop commented Nov 12, 2015

I almost have this completely ready. Just one thing, I don't understand how src/jx/Proxy/console_log.h is working for WINONECORE / WinRT. The file was added here, where it was extracted out of multiple files, but also added DebuggerOutput_ for WinRT, without adding declarations anywhere. The console_log.h header implicitly adds a dependency to stdarg and OutputDebugString, which is broken at least for v8_typed_array.cc, maybe other sources as well. This should be refactored out, but that's a larger change and is independent of the original issue, which is that JX_DefineMainFile isn't executing. I'm going to make a PR to fix just that, and will follow up later with the fix for console.log going to the Debug Output.

@obastemur
Copy link
Member

WinRT uses Chakra only. Chakra uses V8_3_28 proxy and "src/jx/Proxy/V8_3_28/V8Environment.h" includes console_log. So it's available on all the project files

dustmop added a commit that referenced this issue Nov 13, 2015
Detect when jxcore is running in a Windows GUI, and if so, don't use invalid file handles when allocating the js console object. This prevents an exception with the engine startup, which was stopping the source given to JX_DefineMainFile from running.
@dustmop
Copy link
Contributor Author

dustmop commented Nov 14, 2015

I made a PR for this, it's a simple fix, in case you didn't see.

dustmop added a commit that referenced this issue Nov 23, 2015
Detect when running in an environment that doesn't use file descriptors 1 & 2 for stdout and stderr streams, such as Windows GUI applications. Using those descriptors will throw an exception at engine startup, which would prevent the source file given to JX_DefineMainFile from running.
obastemur added a commit that referenced this issue Nov 24, 2015
Fix embedding in a Windows non-console app, issue #608.
@dustmop dustmop closed this as completed Nov 24, 2015
dustmop added a commit that referenced this issue Nov 27, 2015
Detect when running in an environment that doesn't use file descriptors 1 & 2 for stdout and stderr streams, such as Windows GUI applications. Using those descriptors will throw an exception at engine startup, which would prevent the source file given to JX_DefineMainFile from running.
JorisKoster added a commit to JorisKoster/jxcore that referenced this issue Feb 11, 2016
Fix jxcore#608 somehow introduced a broken fix for supporting embedded headless client (like e.g. Windows UI applications). The 'hasStdFds' should indicate whether stdout/stdin/stderr is available, in jxcore#608 however it was true when the handle type for stdout was unknown. Changing this will result in crash when running subengine in a console. Pull request jxcore#836 will solve that issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants