Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Function marked as IO-only was called from a thread that disallows IO #263

Open
geoff-addepar opened this issue Dec 5, 2016 · 3 comments
Labels

Comments

@geoff-addepar
Copy link

I built electron against a debug build of chromium, and running electron with no arguments yields the following assertion failure:

#0 0x7fc100175dee base::debug::StackTrace::StackTrace()
#1 0x7fc1001d642f logging::LogMessage::~LogMessage()
#2 0x7fc10032638f base::ThreadRestrictions::AssertIOAllowed()
#3 0x7fc1001b8761 base::PathExists()
#4 0x7fc0f7d4dbf8 ui::ResourceBundle::GetLocaleFilePath()
#5 0x7fc0f7d4da59 ui::ResourceBundle::LocaleDataPakExists()
#6 0x7fc0f7d22a5e <unknown>
#7 0x7fc0f7d222bc l10n_util::CheckAndResolveLocale()
#8 0x7fc0f7d22db2 <unknown>
#9 0x7fc0f7d22f0b l10n_util::GetApplicationLocale()
#10 0x7fc0f7d22f71 l10n_util::GetApplicationLocale()
#11 0x000000fc6f1d brightray::URLRequestContextGetter::GetURLRequestContext()
#12 0x7fc0f1492b1c content::ChromeAppCacheService::InitializeOnIOThread()
#13 0x7fc0f201aabb <unknown>
#14 0x7fc0f201a926 <unknown>
#15 0x7fc0f201a88b <unknown>
#16 0x7fc0f201a32c <unknown>
#17 0x7fc10015ce1e <unknown>
#18 0x7fc10017b57e base::debug::TaskAnnotator::RunTask()
#19 0x7fc1001f35dc base::MessageLoop::RunTask()
#20 0x7fc1001f3878 base::MessageLoop::DeferOrRunPendingTask()
#21 0x7fc1001f3ad2 base::MessageLoop::DoWork()
#22 0x7fc10013fbae base::MessagePumpLibevent::Run()
#23 0x7fc1001f2fcf base::MessageLoop::RunHandler()
#24 0x7fc100283024 base::RunLoop::Run()
#25 0x7fc10031c445 base::Thread::Run()
#26 0x7fc0f155f526 content::BrowserThreadImpl::IOThreadRun()
#27 0x7fc0f155f8ce content::BrowserThreadImpl::Run()
#28 0x7fc10031c78d base::Thread::ThreadMain()
#29 0x7fc1003077ea <unknown>
#30 0x7fc0f070d70a start_thread
#31 0x7fc0ea49782d clone

Yes, this is the "IO" thread, but it's not supposed to do any actual I/O. See BrowserProcessSubThread::Init.

Chrome/Chromium gets the locale from disk only once during startup, and doesn't call l10n_util::GetApplicationLocale after that point.

@geoff-addepar
Copy link
Author

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
Copy link

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
Copy link
Author

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

@stale stale bot added the wontfix label Mar 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants