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

Crash when trying to import chrome bookmark in debug mode #250

Closed
simonhong opened this issue Jun 1, 2018 · 4 comments
Closed

Crash when trying to import chrome bookmark in debug mode #250

simonhong opened this issue Jun 1, 2018 · 4 comments
Assignees
Labels

Comments

@simonhong
Copy link
Collaborator

@simonhong simonhong commented Jun 1, 2018

[20472:775:0601/162223.743699:ERROR:process_singleton_posix.cc(280)] Failed to create /Users/simonhong/Library/Application Support/Google/Chrome/SingletonLock: File exists (17)
[20472:775:0601/162223.743789:FATAL:thread_restrictions.cc(29)] Check failed: !g_blocking_disallowed.Get().Get(). Function marked as blocking was called from a scope that disallows blocking! If this task is running inside the TaskScheduler, it needs to have MayBlock() in its TaskTraits. Otherwise, consider making this blocking work asynchronous or, as a last resort, you may use ScopedAllowBlocking in a narrow scope.
0   libbase.dylib                       0x000000010c38fb6e base::debug::StackTrace::StackTrace(unsigned long) + 174
1   libbase.dylib                       0x000000010c38fc2d base::debug::StackTrace::StackTrace(unsigned long) + 29
2   libbase.dylib                       0x000000010c38e0ac base::debug::StackTrace::StackTrace() + 28
3   libbase.dylib                       0x000000010c42791c logging::LogMessage::~LogMessage() + 460
4   libbase.dylib                       0x000000010c425665 logging::LogMessage::~LogMessage() + 21
5   libbase.dylib                       0x000000010c68fc6c base::AssertBlockingAllowed() + 188
6   libbase.dylib                       0x000000010c3ed635 base::(anonymous namespace)::CallLstat(char const*, stat*) + 21
7   libbase.dylib                       0x000000010c3f2a74 base::IsLink(base::FilePath const&) + 340
8   libchrome_dll.dylib                 0x000000011bb85caf ProcessSingleton::Create() + 511
9   libchrome_dll.dylib                 0x000000011a51f0c3 ChromeProfileLock::Lock() + 83
10  libchrome_dll.dylib                 0x000000011a51ef69 ChromeProfileLock::ChromeProfileLock(base::FilePath const&) + 361
11  libchrome_dll.dylib                 0x000000011a51f0fd ChromeProfileLock::ChromeProfileLock(base::FilePath const&) + 29
12  libchrome_dll.dylib                 0x000000011a51bd6f BraveExternalProcessImporterHost::CheckForChromeLock(importer::SourceProfile const&) + 351
13  libchrome_dll.dylib                 0x000000011a51ba3c BraveExternalProcessImporterHost::StartImportSettings(importer::SourceProfile const&, Profile*, unsigned short, ProfileWriter*) + 508
14  libchrome_dll.dylib                 0x0000000120b05176 settings::ImportDataHandler::StartImport(importer::SourceProfile const&, unsigned short) + 918
15  libchrome_dll.dylib                 0x0000000120b03dde settings::ImportDataHandler::ImportData(base::ListValue const*) + 2478
@simonhong simonhong closed this Jun 1, 2018
@simonhong simonhong reopened this Jun 1, 2018
@simonhong simonhong changed the title Crash when trying to import chrome bookmark Crash when trying to import chrome bookmark in debug mode Jun 1, 2018
@garrettr
Copy link
Contributor

@garrettr garrettr commented Jun 1, 2018

@simonhong @bbondy Would you consider brave/brave-core@14a1524 to be a sufficient fix for this issue, or would you prefer I spend the extra time refactoring ChromeProfileLock to do blocking operations on the IO thread?

Reasons for quick fix:

  • Works, quick.
  • Same approach used by the Chromium developers in FirefoxProfileLock.
  • The ChromeProfileLock is used infrequently and only in the specific circumstance of a user requesting importing from a Chrome profile, so this will not cause performance problems outside of that specific context. In my manual testing, there is no noticeable "jank" from temporarily allowing IO on the UI thread during Chrome profile import.

Reasons against quick fix:

  • Comments on ScopedAllowIO say its use is "deprecated" and "almost certainly always incorrect."
@simonhong
Copy link
Collaborator Author

@simonhong simonhong commented Jun 4, 2018

IMO, if this doesn't happen frequently and introduce noticeable jank in UI thread, I'm fine with this quick fix.

@bbondy
Copy link
Member

@bbondy bbondy commented Jun 4, 2018

Let's close this with your fix and just post another issue for the Backlog milestone to change it.

@garrettr
Copy link
Contributor

@garrettr garrettr commented Jun 4, 2018

Let's close this with your fix and just post another issue for the Backlog milestone to change it.

Filed #271 as follow-up.

cezaraugusto added a commit to brave/brave-core that referenced this issue Jun 13, 2018
@bbondy bbondy added this to the Releasable builds milestone Jun 14, 2018
@bbondy bbondy added the QA/No label Aug 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.