Skip to content
This repository has been archived by the owner on Sep 2, 2021. It is now read-only.

[NOT READY] fix some linux compile issues with CEF 2171 #489

Closed
wants to merge 25 commits into from

Conversation

jasonsanjose
Copy link
Member

This PR fixes some compile issues, but I'm still seeing errors that I haven't been able to resolve:

jasonsj@ubuntu:~/Github/brackets-shell$ grunt build
Running "build" task

Running "build-linux" task
>> out/Release/obj.target/Brackets/appshell/cefclient_gtk.o: In function `main':
>> cefclient_gtk.cpp:(.text+0x7cc): undefined reference to `CefExecuteProcess(CefMainArgs const&, CefRefPtr<CefApp>, void*)'
>> cefclient_gtk.cpp:(.text+0xb99): undefined reference to `CefInitialize(CefMainArgs const&, CefStructBase<CefSettingsTraits> const&, CefRefPtr<CefApp>, void*)'
>> cefclient_gtk.cpp:(.text+0xff1): undefined reference to `CefBrowserHost::CreateBrowser(CefWindowInfo const&, CefRefPtr<CefClient>, CefStringBase<CefStringTraitsUTF16> const&, CefStructBase<CefBrowserSettingsTraits> const&, CefRefPtr<CefRequestContext>)'
>> out/Release/obj.target/libcef_dll_wrapper/libcef_dll/wrapper/libcef_dll_wrapper.o: In function `CefShutdown()':
>> libcef_dll_wrapper.cc:(.text+0x2ac): undefined reference to `CefCppToC<CefCompletionHandlerCppToC, CefCompletionHandler, _cef_completion_handler_t>::DebugObjCt'
>> libcef_dll_wrapper.cc:(.text+0x365): undefined reference to `CefCToCpp<CefDOMEventCToCpp, CefDOMEvent, _cef_domevent_t>::DebugObjCt'
>> libcef_dll_wrapper.cc:(.text+0x38a): undefined reference to `CefCppToC<CefDOMEventListenerCppToC, CefDOMEventListener, _cef_domevent_listener_t>::DebugObjCt'
>> libcef_dll_wrapper.cc:(.text+0x952): undefined reference to `CefCppToC<CefTraceClientCppToC, CefTraceClient, _cef_trace_client_t>::DebugObjCt'
>> out/Release/obj.target/libcef_dll_wrapper/libcef_dll/wrapper/libcef_dll_wrapper.o: In function `CefGetTraceBufferPercentFullAsync()':
>> libcef_dll_wrapper.cc:(.text+0x12a9): undefined reference to `cef_get_trace_buffer_percent_full_async'
>> out/Release/obj.target/libcef_dll_wrapper/libcef_dll/wrapper/libcef_dll_wrapper.o: In function `CefEndTracingAsync()':
>> libcef_dll_wrapper.cc:(.text+0x12c2): undefined reference to `cef_end_tracing_async'
>> out/Release/obj.target/libcef_dll_wrapper/libcef_dll/wrapper/libcef_dll_wrapper.o: In function `CefCppToC<CefTraceClientCppToC, CefTraceClient, _cef_trace_client_t>::Wrap(CefRefPtr<CefTraceClient>)':
>> libcef_dll_wrapper.cc:(.text._ZN9CefCppToCI20CefTraceClientCppToC14CefTraceClient19_cef_trace_client_tE4WrapE9CefRefPtrIS1_E[_ZN9CefCppToCI20CefTraceClientCppToC14CefTraceClient19_cef_trace_client_tE4WrapE9CefRefPtrIS1_E]+0x4f): undefined reference to `CefTraceClientCppToC::CefTraceClientCppToC(CefTraceClient*)'
>> out/Release/obj.target/libcef_dll_wrapper/libcef_dll/ctocpp/domnode_ctocpp.o: In function `CefCppToC<CefDOMEventListenerCppToC, CefDOMEventListener, _cef_domevent_listener_t>::Wrap(CefRefPtr<CefDOMEventListener>)':
>> domnode_ctocpp.cc:(.text._ZN9CefCppToCI25CefDOMEventListenerCppToC19CefDOMEventListener24_cef_domevent_listener_tE4WrapE9CefRefPtrIS1_E[_ZN9CefCppToCI25CefDOMEventListenerCppToC19CefDOMEventListener24_cef_domevent_listener_tE4WrapE9CefRefPtrIS1_E]+0x4f): undefined reference to `CefDOMEventListenerCppToC::CefDOMEventListenerCppToC(CefDOMEventListener*)'
>> collect2: error: ld returned 1 exit status
>> make: * [out/Release/Brackets] Error 1
Warning: Task "build-linux" failed. Use --force to continue.

Aborted due to warnings.

The closest thing I've found on the forums is http://www.magpcss.org/ceforum/viewtopic.php?f=6&t=10786, but making the suggested change to appshell.gyp, running grunt setup and grunt build again doesn't improve anything.

Another notable bug with 2171 is that the sample cefclient doesn't build with Ubuntu 14.04 https://code.google.com/p/chromiumembedded/issues/detail?id=1430. It's patched later in build 3.2171.1941,

@jasonsanjose
Copy link
Member Author

FYI below is the current error message when building linux on the current jeff/cef_2171_mac branch. This PR fixes these issues, but introduces a new one (see description above).

jasonsj@ubuntu:~/Github/brackets-shell$ grunt build
Running "build" task

Running "build-linux" task
>> In file included from appshell/command_callbacks.h:4:0,
>>                  from appshell/client_handler.h:15,
>>                  from appshell/appshell_extensions.h:27,
>>                  from appshell/appshell_extensions.cpp:24:
>> appshell/appshell_extensions_platform.h: In function ‘void* getMenuParent(CefRefPtr<CefBrowser>)’:
>> appshell/appshell_extensions_platform.h:65:57: error: ‘GTK_WIDGET’ was not declared in this scope
>>          GTK_WIDGET(browser->GetHost()->GetWindowHandle()),
>>                                                          ^
>> appshell/appshell_extensions_platform.h:66:9: error: ‘GTK_TYPE_VBOX’ was not declared in this scope
>>          GTK_TYPE_VBOX);
>>          ^
>> appshell/appshell_extensions_platform.h:66:22: error: ‘gtk_widget_get_ancestor’ was not declared in this scope
>>          GTK_TYPE_VBOX);
>>                       ^
>> make: * [out/Release/obj.target/Brackets/appshell/appshell_extensions.o] Error 1
Warning: Task "build-linux" failed. Use --force to continue.

Aborted due to warnings.

@ingorichter
Copy link
Contributor

let me check this out. Thanks @jasonsanjose.

@jasonsanjose
Copy link
Member Author

Current build error:

jasonsj@ubuntu:~/Github/brackets-shell$ grunt build
Running "build" task

Running "build-linux" task
>> /usr/bin/ld: out/Release/obj.target/libcef_dll_wrapper/libcef_dll/base/cef_lock_impl.o: undefined reference to symbol 'pthread_mutexattr_settype@@GLIBC_2.2.5'
>> //lib/x86_64-linux-gnu/libpthread.so.0: error adding symbols: DSO missing from command line
>> collect2: error: ld returned 1 exit status
>> make: * [out/Release/Brackets] Error 1
Warning: Task "build-linux" failed. Use --force to continue.

Aborted due to warnings.

@Mark-Simulacrum
Copy link
Contributor

I'm currently getting the following upon launching Brackets, and selecting index.html:

[1205/172059:ERROR:browser_main_loop.cc(163)] Running without the SUID sandbox! See https://code.google.com/p/chromium/wiki/LinuxSUIDSandboxDevelopment for more information on developing with the sandbox on.
[1205/172059:ERROR:browser_main_loop.cc(209)] Gdk: /build/buildd/gtk+2.0-2.24.23/gdk/x11/gdkdrawable-x11.c:952 drawable is not a pixmap or window
[1205/172059:ERROR:renderer_main.cc(207)] Running without renderer sandbox
[1205/172059:ERROR:sandbox_linux.cc(305)] InitializeSandbox() called with multiple threads in process gpu-process
[1]    19622 segmentation fault  ./Brackets

@peterflynn
Copy link
Member

@Mark-Simulacrum Hmm, interesting... someone hit that recently when trying to run generic cefclient too: adobe/brackets#6427 (comment).

These Chromium docs imply Chromium expects to find a root-owned special permissions executable in /usr/local/sbin/chrome-devel-sandbox... that would be a pain for us to try to auto-install, but I feel like we shouldn't have to: the name alone implies that it's a utility intended only for use in a dev environment, not for end users. I wonder what we could do instead? That docs page says only "Package maintainers should make sure the setuid binary is installed and defined in GYP as linux_sandbox_path." (@ingorichter, does that line make any sense to you?)

@ingorichter
Copy link
Contributor

Yes, I see the same issue. I see two Windows in the beginning and then Brackets crashes. I can't find the core dumps on my Ubuntu 14.10 system. @Mark-Simulacrum do you know where they hide?

@peterflynn
Copy link
Member

In CEF bug #524, Marshall says "Linux: For binary distributions a new chrome-sandbox executable with SUID permissions must be placed next to the CEF executable." Odd that their own cefclient distros seem to be failing to do that correctly, though... bug on their end, maybe?

@Mark-Simulacrum
Copy link
Contributor

@ingorichter No, and I can't seem to find anything about core dumps. I haven't done much development though. Will post back if I find more information.

@ingorichter
Copy link
Contributor

The funny thing is, that we have disabled sandboxing on Windows, OSX and Linux. The docs recommended this (if you are certain to understand what you are doing by disabling this security feature).
I'd like to have a look at the core dump to rule out any other issue. Usually Chrome shouldn't crash when running without the sandbox executable.

@Mark-Simulacrum
Copy link
Contributor

I may have found the way to access the coredump, but I'm not completely certain. Execute ulimit -c unlimited (sets core file size to unlimited) and then attempt to reproduce the bug. A core file should show up in the current working directory.

@ingorichter
Copy link
Contributor

Thanks @Mark-Simulacrum! I have the same issues with my changes. The main issues at the moment are

  • There are two different ways to quit Brackets
    1. Closing the window
    2. Calling Exit from the file menu

In both cases there are different code paths for each method. They have some codepath in common, but they are getting triggered in a different way. But this is only, because I haven't been able so far to trigger the window destruction from CloseWindow (appshell_extensions_gtk.cpp).

  • CefQuitMessageLoop() doesn't quit Brackets
    This seems to stop the CEF message loop, but CefShutdown hangs and will never return.
    I don't know how to hook into this whole CEF lifecycle. I thought I understood it, but then I got proven wrong again and again and again....

It seems that we need to trigger the destruction of the GTK window. This would give us a chance to run the same function that is called when the window gets closed.
Everytime I send an event to that GTK Window, it ends up in a SEGFAULT. But how should we trigger the window destruction? I'm running dry on options. I created flow charts to visualize it to me, but it seems there is something important missing. The CEF test app wasn't a big help either so far.

@Mark-Simulacrum
Copy link
Contributor

@ingorichter The way I understand CefQuitMessageLoop() from all that I have read on the subject also does not correspond to what I see during experimentation. From what I can tell, it seems that the only (valid) reason for CefQuitMessageLoop not working is it being in a different Thread than the CefRunMessageLoop call in cefclient_gtk.cpp. I'm still fairly new to C++, does the thread theory seem worth investigating?

@ingorichter
Copy link
Contributor

@Mark-Simulacrum I have tried this, but it didn't work either. This was only a guess, since this shouldn't be required to terminate the app. The test app uses CEF_REQUIRE_UI_THREAD() in ClientHandler::OnBeforeClose and that is the method that terminates the test app. But in my case this didn't work. :-(

@Mark-Simulacrum
Copy link
Contributor

@ingorichter I can't come up with anything else to try. Will keep you posted if I think of anything, and will happily test anything that comes up. Sorry about my novelty to CEF/GTK/C++.

@ingorichter
Copy link
Contributor

@Mark-Simulacrum No worries. All help and ideas are welcome at the moment. Sometimes the most unlikely ones are leading to success. Consider this a great learning experience.

@nethip
Copy link
Contributor

nethip commented Dec 11, 2014

@ingorichter @Mark-Simulacrum I tried building this branch and ended up with the same errors which @jasonsanjose had got. To fix these, I have added pthread library to appshell.gyp file. That seemed to fix the issue and I was able to launch Brackets on Linux. I spent some time in understanding the difference between CefClient and Brackets esp cefclient_gtk.cpp. After numerous hours of investigation, I found that commenting startNodeProcess() in int main() of cefclient_gtk.cpp is taking away the crash! Looks like the pThread creation inside startNodeProcess() is creating the problem. May be we can try to terminate this thread before CefShutDown().

Apart from this change I had to make another change. In this branch as CefQuitMessageLoop() was not getting called from anywhere, I explicitly added this call in OnBeforeClose(). The function then looked like this.

void ClientHandler::OnBeforeClose(CefRefPtr<CefBrowser> browser) {
  REQUIRE_UI_THREAD();

  g_message("OnBeforeClose called");
  if (CanCloseBrowser(browser)) {
    if (m_BrowserId == browser->GetIdentifier()) {
      // Free the browser pointer so that the browser can be destroyed
      m_Browser = NULL;
    }

    browser_window_map_.erase((ClientWindowHandle)browser->GetHost()->GetWindowHandle());
  }

// As m_quitting was not being set on this experimental branch I had to explicitly
// call CefQuitMessageLoop() here.
/*
  if (m_quitting) {
    g_message("Call DispatchCloseToNextBrowser()");
    DispatchCloseToNextBrowser();

    AfterClose();
  }
*/
CefQuitMessageLoop();
}

The other change(which may be minor) is that I have called gtk_init() after CefInitialize() (That is how it was getting done in CefClient).

After these changes I could quit Brackets with one click on the close button or using Ctrl+Q. However even with these changes, I could still see a core dump, if I launch from the terminal or using gdb. But at least it is quitting :) and we can look at the core dump problem as a separate issue.

Hope this helps!

@nethip
Copy link
Contributor

nethip commented Dec 12, 2014

@ingorichter @Mark-Simulacrum I am able to dismiss the crash by calling startNodeProcess() before CefInitialize(). This will now quit Brackets without crash. However I found something more interesting today. While I was looking at the fork() inside startNodeProcess() I found that the main process(which is forking the child process which in turn launches Node process) was quitting after spawning the child process unexpectedly and looks like the UI is now running on the forked process. I came to this conclusion by checking pids and ppids of list of processes spawned by Brackets and its sub processes in the activity monitor. I monitored the pid of the Brackets process before the open html dialog popups and pids of the other processes spawned after CreateBrowser(). This might even explain why @ingorichter is getter SEGFAULTs on sending a terminate to the main window.

So I am not sure if just by moving startNodeProcess() before CefInitialize is the solution. The solution might involve delving more into creation of pthread and fork.

@nethip
Copy link
Contributor

nethip commented Dec 15, 2014

My theory, that I mentioned above, does not seem to hold good. This seems to be intermittent. Today I am not able to reproduce what I had observed on Friday 😞

However I tried moving startNodeProcess() to the beginning of int main() and things seem to be working fine. I did some basic unit testing.

One issue I found in my unit testing is that, upon opening a new window and closing that window, the entire application quits(along with the old window). I am thinking, this may be because of some experimental changes that are present in this branch.

@ingorichter
Copy link
Contributor

@nethip Thank you for investigating this issue. I think the code that closed the window doesn't take into account that there might be more than one window left. This is part of this experiment and needs to be fixed. I wonder if it's a good idea to start the node process before everything else is ready. What happens, if Brackets can't start up properly, then we could end up with this lonely node process.
After the release, I'm going back to find a solution for the issues that we are seeing right now. Anyway, thank you very much for your support.

@busykai
Copy link

busykai commented Jan 7, 2015

@ingorichter, what is the difference between isReallyClosing (file-level in cefclient_gtk.cpp) and ClientHandler::m_quitting? Aren't they serving the same purpose -- to mark the second call to close handler as actually allowing to close. Pardon if it's obvious, just starting here.

@nethip
Copy link
Contributor

nethip commented Jan 7, 2015

@busykai isReallyClosing was added to experiment and solve the crash on quit on Linux. I think isReallyClosing is used to figure out if a call to destroy() orginated from terminate() (in which case CefMessageLoop is explicitly called). This should eventually be moved to ClientHandler::OnBeforeClose()(like how it happens on other platforms). Please see my comments above for a more detailed explanation.

Add -lpthread to LIBS (libraries).
@busykai
Copy link

busykai commented Jan 8, 2015

thanks, @nethip! so i've gotten to the point where the shell would quit from the message loop, but it started to hang in shutdown. looking at to threads state in the hang, i found that nodeReadThread was in the blocking read, rewrote it to use select, now the thread quits, but the hang still appears. apparently, the shutdown waits for the renderer thread. here is the state of threads and the main thread stack trace:

(gdb) info threads
  Id   Target Id         Frame 
  8    Thread 0x7fb9fef14700 (LWP 15499) "dconf worker" 0x00007fba0a523bad in poll ()
    at ../sysdeps/unix/syscall-template.S:81
  7    Thread 0x7fb9fe713700 (LWP 15500) "gdbus" 0x00007fba0a523bad in poll ()
    at ../sysdeps/unix/syscall-template.S:81
  6    Thread 0x7fb9fce7a700 (LWP 15501) "sandbox_ipc_thr" 0x00007fba0a523bad in poll ()
    at ../sysdeps/unix/syscall-template.S:81
  5    Thread 0x7fb9fbe78700 (LWP 15504) "inotify_reader" 0x00007fba0a528823 in select ()
    at ../sysdeps/unix/syscall-template.S:81
  4    Thread 0x7fb9ec944700 (LWP 15545) "Blink Heap Mark" pthread_cond_wait@@GLIBC_2.3.2
    () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
  3    Thread 0x7fb9ec143700 (LWP 15546) "Blink Heap Mark" pthread_cond_wait@@GLIBC_2.3.2
    () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
  2    Thread 0x7fb9eb942700 (LWP 15547) "Blink GC Sweepe" pthread_cond_wait@@GLIBC_2.3.2
    () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
* 1    Thread 0x7fba1075ca00 (LWP 15498) "Brackets" 0x00007fba0ad1f66b in pthread_join (
    threadid=140436788717312, thread_return=0x0) at pthread_join.c:92
(gdb) where 
#0  0x00007fba0ad1f66b in pthread_join (threadid=140436788717312, thread_return=0x0)
    at pthread_join.c:92
#1  0x00007fba0cdbed5f in base::PlatformThread::Join(base::PlatformThreadHandle) ()
   from /home/kai/src/brackets-shell/out/Release/lib/libcef.so
#2  0x00007fba0cdc26a0 in base::SimpleThread::Join() ()
   from /home/kai/src/brackets-shell/out/Release/lib/libcef.so
#3  0x00007fba0eed6529 in content::RenderSandboxHostLinux::~RenderSandboxHostLinux() ()
   from /home/kai/src/brackets-shell/out/Release/lib/libcef.so
#4  0x00007fba0eed6575 in Singleton<content::RenderSandboxHostLinux, DefaultSingletonTraits<content::RenderSandboxHostLinux>, content::RenderSandboxHostLinux>::OnExit(void*) ()
   from /home/kai/src/brackets-shell/out/Release/lib/libcef.so
#5  0x00007fba0cd688b0 in base::AtExitManager::ProcessCallbacksNow() ()
   from /home/kai/src/brackets-shell/out/Release/lib/libcef.so
#6  0x00007fba0cd68803 in base::AtExitManager::~AtExitManager() ()
   from /home/kai/src/brackets-shell/out/Release/lib/libcef.so
#7  0x00007fba0f51acf6 in content::ContentMainRunnerImpl::Shutdown() ()
   from /home/kai/src/brackets-shell/out/Release/lib/libcef.so
#8  0x00007fba0cc9e053 in CefContext::FinalizeShutdown() ()
   from /home/kai/src/brackets-shell/out/Release/lib/libcef.so
#9  0x00007fba0cc9daaf in CefShutdown() ()
   from /home/kai/src/brackets-shell/out/Release/lib/libcef.so
#10 0x0000000000433654 in CefShutdown() ()
#11 0x000000000042f574 in main ()

I don't have much knowledge of CEF/Chromium internals, so learning as I go. I wonder where the renderer thread went...

@nethip
Copy link
Contributor

nethip commented Jan 8, 2015

Thanks @busykai for looking into this. Sure this is a great learning experience for all of us. In case you didn't know, Brackets uses CEF3 which has a multi-process architecture. All of the java script execution happens in a different process called render process. The name of the render process for Brackets is again Brackets on Linux (that is why you see more than one Brackets processes in the Activity Monitor). So I am not sure if render thread is what the main thread is waiting on. Is it possible for you to figure out which thread the main thread is waiting on?

While I was experimenting on this branch I found out that, if I call startNodeProcess() before CefInitialize(), this is taking away the crash. But as @ingorichter mentioned, this means that, if there is any issue with the main Brackets process, we might end up with an orphan (node) process.

@ingorichter I have a question on calling startNodeProcess() before CefInitialize().Will the ping pong logic that we had implemented to check if both are alive take care of the orphan process situation, in case the parent dies before the child?

And from the above call stack do we suspect anything with disabling sandboxing?

@ingorichter
Copy link
Contributor

I don't think that the ping pong logic takes care of orphaned threads. I still think it's the best solution to wait before calling startNodeProcess to avoid this orphan situation. I don't know how likely this will happen, but this was working before and we should figure out what made it stop working.
Sandboxing was never enabled in our builds. We have explicitly turned it off. It might be good or bad. ;-)
As an experiment, we could try to not start the node process at all to see if this has any impact on the shutdown of Brackets. This way we could narrow the scope at bit. What do you think?

@nethip
Copy link
Contributor

nethip commented Jan 9, 2015

@ingorichter If the ping pong does not take care of the orphan process problem then we should place of calling startNodeProcess().

About not calling startNodeProcess, I did that and it took away the crash. Brackets would terminate gracefully. I am pretty much sure it has something to do with statNodeProcess itself. Either the thread or the way we are forking or something else that I am not able to catch.

@busykai Did you get a chance to look at this issue?

@jasonsanjose
Copy link
Member Author

Closing. Will create a new PR.

@jasonsanjose
Copy link
Member Author

Updated PR with cleaned up branch here #499

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants