Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2206973002:
Fix xcb crash when creating gl context (Closed)
DescriptionFix xcb crash when creating gl context
This is a regression introduced by
https://codereview.chromium.org/2139673002. GPU process crashed due an
assertion: '!xcb_xlib_threads_sequence_lost' failed.
BUG=628823
Committed: https://crrev.com/a1d7428578d800f6e0b474dd07c5c760021ff1e1
Cr-Commit-Position: refs/heads/master@{#409814}
Patch Set 1 #Patch Set 2 : sync firstly #Messages
Total messages: 16 (3 generated)
qiankun.miao@intel.com changed reviewers: + cwallez@chromium.org, kbr@chromium.org, piman@chromium.org, yunchao.he@intel.com, zmo@chromium.org
This CL fixed the crash issue locally and no any error in try bots. PTAL.
On 2016/08/03 at 10:53:48, qiankun.miao wrote: > This CL fixed the crash issue locally and no any error in try bots. PTAL. Thanks for figuring out what caused the crash. However this error suppression is important and not the root cause: the issue must be that this code is executed before XInitThreads is called (see InitializeThreadedX11?).
On 2016/08/03 14:45:22, Corentin Wallez wrote: > On 2016/08/03 at 10:53:48, qiankun.miao wrote: > > This CL fixed the crash issue locally and no any error in try bots. PTAL. > > Thanks for figuring out what caused the crash. However this error suppression is > important and not the root cause: the issue must be that this code is executed > before XInitThreads is called (see InitializeThreadedX11?). Corentin, please feel free to pick this bug. Let's fix the crash ASAP.
On 2016/08/03 at 16:21:17, qiankun.miao wrote: > On 2016/08/03 14:45:22, Corentin Wallez wrote: > > On 2016/08/03 at 10:53:48, qiankun.miao wrote: > > > This CL fixed the crash issue locally and no any error in try bots. PTAL. > > > > Thanks for figuring out what caused the crash. However this error suppression is > > important and not the root cause: the issue must be that this code is executed > > before XInitThreads is called (see InitializeThreadedX11?). > > Corentin, please feel free to pick this bug. Let's fix the crash ASAP. Will do, do you have hints as to the best way to repro this?
On 2016/08/03 17:39:07, Corentin Wallez wrote: > On 2016/08/03 at 16:21:17, qiankun.miao wrote: > > On 2016/08/03 14:45:22, Corentin Wallez wrote: > > > On 2016/08/03 at 10:53:48, qiankun.miao wrote: > > > > This CL fixed the crash issue locally and no any error in try bots. PTAL. > > > > > > Thanks for figuring out what caused the crash. However this error > suppression is > > > important and not the root cause: the issue must be that this code is > executed > > > before XInitThreads is called (see InitializeThreadedX11?). > > > > Corentin, please feel free to pick this bug. Let's fix the crash ASAP. > > Will do, do you have hints as to the best way to repro this? Per Qiankun's comments on https://bugs.chromium.org/p/chromium/issues/detail?id=628823 , it should happen on Linux with Intel GPUs just by refreshing some of the test pages many times.
On 2016/08/03 at 20:06:21, kbr wrote: > On 2016/08/03 17:39:07, Corentin Wallez wrote: > > On 2016/08/03 at 16:21:17, qiankun.miao wrote: > > > On 2016/08/03 14:45:22, Corentin Wallez wrote: > > > > On 2016/08/03 at 10:53:48, qiankun.miao wrote: > > > > > This CL fixed the crash issue locally and no any error in try bots. PTAL. > > > > > > > > Thanks for figuring out what caused the crash. However this error > > suppression is > > > > important and not the root cause: the issue must be that this code is > > executed > > > > before XInitThreads is called (see InitializeThreadedX11?). > > > > > > Corentin, please feel free to pick this bug. Let's fix the crash ASAP. > > > > Will do, do you have hints as to the best way to repro this? > > Per Qiankun's comments on https://bugs.chromium.org/p/chromium/issues/detail?id=628823 , it should happen on Linux with Intel GPUs just by refreshing some of the test pages many times. Qiankun, I am not able to repro this on Intel Linux with top of tree Chrome. What distribution did you use to repro?
On 2016/08/03 at 21:21:10, Corentin Wallez wrote: > On 2016/08/03 at 20:06:21, kbr wrote: > > On 2016/08/03 17:39:07, Corentin Wallez wrote: > > > On 2016/08/03 at 16:21:17, qiankun.miao wrote: > > > > On 2016/08/03 14:45:22, Corentin Wallez wrote: > > > > > On 2016/08/03 at 10:53:48, qiankun.miao wrote: > > > > > > This CL fixed the crash issue locally and no any error in try bots. PTAL. > > > > > > > > > > Thanks for figuring out what caused the crash. However this error > > > suppression is > > > > > important and not the root cause: the issue must be that this code is > > > executed > > > > > before XInitThreads is called (see InitializeThreadedX11?). > > > > > > > > Corentin, please feel free to pick this bug. Let's fix the crash ASAP. > > > > > > Will do, do you have hints as to the best way to repro this? > > > > Per Qiankun's comments on https://bugs.chromium.org/p/chromium/issues/detail?id=628823 , it should happen on Linux with Intel GPUs just by refreshing some of the test pages many times. > > Qiankun, I am not able to repro this on Intel Linux with top of tree Chrome. What distribution did you use to repro? Also adding logging to Chrome, we can see that XInitThreads is called before XSetErrorHandler in all cases, see https://codereview.chromium.org/2208743005 it does not exclude another part of the code calling X11 functions on the same Display, without having called XInitThreads.
I updated the CL. Do XSync before doing XSetErrorHandler to make sure all generated errors are processed. Please take a look at the new CL.
On 2016/08/04 at 13:00:19, qiankun.miao wrote: > I updated the CL. Do XSync before doing XSetErrorHandler to make sure all generated errors are processed. > > Please take a look at the new CL. LGTM. Thank you for finding such a good fix, X can be quite difficult! You still need OWNERS' review though.
Fantastic, Qiankun! Thank you for finding this fix. LGTM!
The CQ bit was checked by qiankun.miao@intel.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Fix xcb crash when creating gl context This is a regression introduced by https://codereview.chromium.org/2139673002. GPU process crashed due an assertion: '!xcb_xlib_threads_sequence_lost' failed. BUG=628823 ========== to ========== Fix xcb crash when creating gl context This is a regression introduced by https://codereview.chromium.org/2139673002. GPU process crashed due an assertion: '!xcb_xlib_threads_sequence_lost' failed. BUG=628823 Committed: https://crrev.com/a1d7428578d800f6e0b474dd07c5c760021ff1e1 Cr-Commit-Position: refs/heads/master@{#409814} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/a1d7428578d800f6e0b474dd07c5c760021ff1e1 Cr-Commit-Position: refs/heads/master@{#409814} |
