Function marked as IO-only was called from a thread that disallows IO about brightray

[geoff-addepar]

As a more general comment, I think it would be useful if the debug build of electron included a version of libchromiumcontent and brightray built in debug mode. There are a number of useful checks, including many related to thread-safety, that I would expect developers working on electron to want to have enabled.

[groundwater]

Hi @geoff-addepar, I definitely like your idea about having debug builds available for libcc and brightray. I think the component builds are build with debugging information, but I'd want to confirm that.

Regarding your posted issue, did you build chromium using libchromiumcontent or another way? The libcc build process patches chromium.

[geoff-addepar]

I built a debug version of libchromiumcontent, and used that to build a debug version of electron/brightray. Note that "with debugging information" (i.e. symbols) is different from what I want: I want builds where DCHECK is implemented, NDEBUG is not defined, and things like CHROME_IPC_LOGGING work. The additional checks available in this type of build help find bugs earlier in the development cycle.

Building this required a few changes to libcc:

diff --git a/chromiumcontent/chromiumcontent.gyp b/chromiumcontent/chromiumcontent.gyp
index 9a311fe..024cc31 100644
--- a/chromiumcontent/chromiumcontent.gyp
+++ b/chromiumcontent/chromiumcontent.gyp
@@ -37,6 +37,7 @@
         '<(DEPTH)/content/content.gyp:content',
         '<(DEPTH)/content/content.gyp:content_app_both',
         '<(DEPTH)/content/content_shell_and_tests.gyp:content_shell_pak',
+        '<(DEPTH)/media/media.gyp:media',
         '<(DEPTH)/net/net.gyp:net_with_v8',
         '<(DEPTH)/ppapi/ppapi_internal.gyp:ppapi_host',
         '<(DEPTH)/ppapi/ppapi_internal.gyp:ppapi_proxy',
@@ -46,6 +47,7 @@
         '<(DEPTH)/third_party/widevine/cdm/widevine_cdm.gyp:widevinecdmadapter',
         '<(DEPTH)/third_party/widevine/cdm/widevine_cdm.gyp:widevine_cdm_version_h',
         '<(DEPTH)/ui/events/events.gyp:dom_keycode_converter',
+        '<(DEPTH)/url/url.gyp:url_lib'
       ],
       'sources': [
         'empty.cc',
diff --git a/script/create-dist b/script/create-dist
index 6a5a86d..ec481ba 100755
--- a/script/create-dist
+++ b/script/create-dist
@@ -395,7 +395,8 @@ def copy_binaries(target_arch, component, output_dir):
     # be used for Release build.
     if component == 'shared_library':
       for library in glob.glob(os.path.join(target_dir, match)):
-        run_strip(target_arch, library)
+        #run_strip(target_arch, library)
+        pass
 
 
 def copy_generated_sources(target_arch, component, output_dir):
diff --git a/script/lib/config.py b/script/lib/config.py
index 67631d1..f034288 100644
--- a/script/lib/config.py
+++ b/script/lib/config.py
@@ -23,7 +23,7 @@ def get_output_dir(target_arch, component):
 
 
 def get_configuration(target_arch):
-  config = 'Release'
+  config = 'Debug'
   if target_arch == 'x64' and sys.platform in ['win32', 'cygwin']:
     config += '_x64'
   return config

Then I followed the instructions in the last comment of electron/electron#2023 to make my electron build pick up my modified binaries. Two things to note:

• I had to run git submodule foreach --recursive git clean -fdx in my electron directory before bootstrapping so that the bootstrap script would actually download the new files
• I built libcc using script/build -t x64 -c shared_library before running create-dist

On the electron side, I also had to make a few changes just to get it to build:

diff --git a/atom/common/native_mate_converters/callback.h b/atom/common/native_mate_converters/callback.h
index decc36e..8d15361 100644
--- a/atom/common/native_mate_converters/callback.h
+++ b/atom/common/native_mate_converters/callback.h
@@ -53,7 +53,7 @@ struct V8FunctionInvoker<v8::Local<v8::Value>(ArgTypes...)> {
     v8::Local<v8::Function> holder = function.NewHandle(isolate);
     v8::Local<v8::Context> context = holder->CreationContext();
     v8::Context::Scope context_scope(context);
-    std::vector<v8::Local<v8::Value>> args = { ConvertToV8(isolate, raw)... };
+    std::vector<v8::Local<v8::Value>> args = std::vector<v8::Local<v8::Value>>{ ConvertToV8(isolate, raw)... };
     v8::Local<v8::Value> ret(holder->Call(holder, args.size(), &args.front()));
     return handle_scope.Escape(ret);
   }
@@ -73,7 +73,7 @@ struct V8FunctionInvoker<void(ArgTypes...)> {
     v8::Local<v8::Function> holder = function.NewHandle(isolate);
     v8::Local<v8::Context> context = holder->CreationContext();
     v8::Context::Scope context_scope(context);
-    std::vector<v8::Local<v8::Value>> args = { ConvertToV8(isolate, raw)... };
+    std::vector<v8::Local<v8::Value>> args = std::vector<v8::Local<v8::Value>>{ ConvertToV8(isolate, raw)... };
     holder->Call(holder, args.size(), &args.front());
   }
 };
@@ -93,7 +93,7 @@ struct V8FunctionInvoker<ReturnType(ArgTypes...)> {
     v8::Local<v8::Function> holder = function.NewHandle(isolate);
     v8::Local<v8::Context> context = holder->CreationContext();
     v8::Context::Scope context_scope(context);
-    std::vector<v8::Local<v8::Value>> args = { ConvertToV8(isolate, raw)... };
+    std::vector<v8::Local<v8::Value>> args = std::vector<v8::Local<v8::Value>>{ ConvertToV8(isolate, raw)... };
     v8::Local<v8::Value> result;
     auto maybe_result =
         holder->Call(context, holder, args.size(), &args.front());

And in vendor/brightray:

diff --git a/brightray.gypi b/brightray.gypi
index 064ae1a..48466d8 100644
--- a/brightray.gypi
+++ b/brightray.gypi
@@ -94,8 +94,6 @@
         'defines': [
           # Used by content_browser_client.h:
           'ENABLE_WEBRTC',
-          # We are using Release version libchromiumcontent:
-          'NDEBUG',
           # Needed by gin:
           'V8_USE_EXTERNAL_STARTUP_DATA',
           # From skia_for_chromium_defines.gypi:
@@ -197,6 +195,7 @@
           'SKIA_DLL',
           'USING_V8_SHARED',
           'WEBKIT_DLL',
+          '_GLIBCXX_DEBUG',
         ],
         'msvs_settings': {
           'VCCLCompilerTool': {
@@ -209,6 +208,9 @@
       },  # Debug_Base
       'Release_Base': {
         'abstract': 1,
+        'defines': [
+          'NDEBUG',
+        ],
         'msvs_settings': {
           'VCCLCompilerTool': {
             'RuntimeLibrary': '0',  # /MT (nondebug static)

Once you've done all that, you can start up out/D/electron and watch it fail.

Note that this bug is not the only problem that shows up when running a debug build. If you get past that issue, here are the next three you will hit:

• atom::NativeWindowViews::NativeWindowViews() calls ui::SetAtomArrayProperty() with an empty list, and that causes DCHECK(!value.empty()); to fail
• atom::AtomNetworkDelegate extends NonThreadSafe. It is constructed on the "electron" thread and used on the IO thread. That causes Check failed: CalledOnValidThread().
• Check failed: handle_scope_implementer->GetMicrotasksScopeDepth() || !handle_scope_implementer->DebugMicrotasksScopeDepthIsZero(). in atom::NodeBindings::LoadEnvironment