Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(43)

Issue 4063041: Fix race condition between ibus_bus_set_global_engine() and ibus_bus_get_global_engine(). (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 10 months ago by Yusuke Sato
Modified:
13 years, 10 months ago
Reviewers:
shawn.p.huang, Peng Huang
CC:
satorux
Base URL:
git@github.com:ibus/ibus.git@master
Visibility:
Public.

Description

Fix race condition between ibus_bus_set_global_engine() and ibus_bus_get_global_engine(). If focus moves between the two API calls, ibus_bus_get_global_engine() might return an unexpected engine name: 1. context A is focused, and the current global engine is "X". 2. ibus_bus_set_global_engine("Y") is called. 3. a user moves the focus from A to B. First, A's engine is set to NULL in bus_ibus_impl_set_focused_context(). Then, in the same function, B's engine is set to "X" (not "Y") since the _ibus_set_global_engine asynchronous call is not finished yet. 4. ibus_bus_set_global_engine("Y") async call successfully finishes. Context A's (not B's) engine is set to "Y", but context B, which has a focus, is not updated. 5. ibus_bus_get_global_engine() is called. expected: Y is returned. actual: X is returned. Since the context B has a focus, and B's engine is X. BUG=http://crosbug.com/11031 TEST=see the bug

Patch Set 1 : review #

Total comments: 2

Patch Set 2 : review fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+34 lines, -4 lines) Patch
M bus/ibusimpl.c View 1 2 chunks +34 lines, -4 lines 0 comments Download

Messages

Total messages: 4
Yusuke Sato
13 years, 10 months ago (2011-01-18 11:59:15 UTC) #1
Peng
http://codereview.appspot.com/4063041/diff/2001/bus/ibusimpl.c File bus/ibusimpl.c (right): http://codereview.appspot.com/4063041/diff/2001/bus/ibusimpl.c#newcode1642 bus/ibusimpl.c:1642: if (ibus->use_global_engine && (context != ibus->focused_context)) { Could you ...
13 years, 10 months ago (2011-01-18 15:02:38 UTC) #2
Yusuke Sato
http://codereview.appspot.com/4063041/diff/2001/bus/ibusimpl.c File bus/ibusimpl.c (right): http://codereview.appspot.com/4063041/diff/2001/bus/ibusimpl.c#newcode1642 bus/ibusimpl.c:1642: if (ibus->use_global_engine && (context != ibus->focused_context)) { We don't ...
13 years, 10 months ago (2011-01-19 01:43:11 UTC) #3
Peng
13 years, 10 months ago (2011-01-19 01:54:54 UTC) #4
lgtm
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b