• Stephan Bergmann's avatar
    Avoid race between DbusIpcThread::close and DbusIpcThread::execute · 38081c08
    Stephan Bergmann yazdı
    ...that caused Flatpak'ed LO to sometimes not terminate properly after
    243d743d "solenv/flatpak-manifest.in:
    incorporate upstream sandboxing improvements" had removed --socket=session-bus
    and thus introduced flatpak-dbus-proxy into the mix:
    
    > Oct 24 15:25:16 <sberg_> I'm not sure who's at fault there; on the LO side I
    >  have a thread processing incoming dbus requests (DbusIpcThread::execute,
    >  <https://opengrok.libreoffice.org/xref/core/desktop/source/app/officeipcthread.cxx#552>);
    >  that thread is typically waiting within dbus_connection_read_write_dispatch
    >  for new messages; if LO wants to close, it needs to get that thread out of
    >  dbus_connection_read_write_dispatch, and the only way I found back then is
    >  what's in DbusIpcThread::close (just following DbusIpcThread::execute), with
    >  a big "this apparently needs a more DBus-y design anyway" testament to my
    >  cluelesness re dbus; it calls dbus_bus_get_private to connect to itself, and
    >  send a Close message that'll cause DbusIpcThread::execute to fall out of the
    >  loop; the dbus_bus_get_private leads to a flatpak_proxy_incoming in the
    >  proxy, which returns TRUE, and after that I see no further activity in the
    >  proxy (but not sure what functions I should all put breakpoints on), and the
    >  LO side is hung in dbus_bus_release_name a few lines futher down
    > Oct 24 15:26:20 <alexlarsson> sberg_: i ran this:
    >  https://github.com/sgh/dbus-examples/blob/master/dbus-signal.c
    > Oct 24 15:26:27 <alexlarsson> sberg_: which calls dbus_bus_release_name
    > Oct 24 15:26:30 <alexlarsson> and it seems to work for me
    > Oct 24 15:26:39 <alexlarsson> If i --own-name=org.DBusTest.SignalTest
    > Oct 24 15:27:50 <sberg_> so maybe it's related to my unorthodox way of trying
    >  to quit the dbus loop there;  if anybody with a clue about dbus would be
    >  willing to help me with the above, I'd be very grateful :)
    > Oct 24 15:28:06 <alexlarsson> I don't see how the proxy should affect it tho
    > Oct 24 15:28:11 <alexlarsson> other than maybe timing?
    > Oct 24 15:29:06 <sberg_> maybe; I think I've seen things actually succeed once
    >  when I stepped through the proxy somewhat manually
    > Oct 24 15:31:01 <alexlarsson> sberg_: eeeentresting
    > Oct 24 15:31:07 <alexlarsson> sberg_: so, it does an own-call
    > Oct 24 15:31:13 <alexlarsson> sberg_: maybe that is what breaks?
    > Oct 24 15:31:33 <sberg_> yeah, that's my dumb speculation
    > Oct 24 15:34:38 <alexlarsson> So, you send the close from another thread, not
    >  waiting for the reply
    > Oct 24 15:34:56 <alexlarsson> then you start working on the main connection
    >  immediately
    > Oct 24 15:35:02 <alexlarsson> shouldn
    > Oct 24 15:35:25 <alexlarsson> shouldn't you grab the mutex after that?
    > Oct 24 15:36:35 <alexlarsson> This whole thing smells racy
    > Oct 24 15:38:18 <alexlarsson> My guess is that with regular dbus, the flush
    >  call will schedule dbus daemon which sends back the message, waking up the
    >  other thread before continuing
    > Oct 24 15:38:22 <sberg_> alexlarsson, ah, yeah, I see; apparently didn't
    >  occur to me back then that I mustn't call dbus_bus_release_name when the
    >  execute loop may still be busy
    > Oct 24 15:38:25 <alexlarsson> But now you have two context switches
    > Oct 24 15:38:37 <alexlarsson> so we continue before we get the reply
    > Oct 24 15:38:44 <alexlarsson> eh, not reply
    > Oct 24 15:38:46 <alexlarsson> the close message
    > Oct 24 15:38:52 <alexlarsson> and then you race
    > Oct 24 15:39:11 <alexlarsson> Honestly i don't remember the exact rules for
    >  dbus threadedness
    > Oct 24 15:39:58 <sberg_> ...so looks like something that can be corrected on
    >  the LO side after all; I'll give it a try
    > Oct 24 15:40:17 <alexlarsson> you're calling dbus_threads_init_default(), so
    >  it should be nominally threadsafe
    > Oct 24 15:40:29 <alexlarsson> still, I imagine it can still deadlock
    > Oct 24 15:43:12 <alexlarsson> sberg_: i still don't see the exact deadlock
    >  though
    > Oct 24 15:43:32 <alexlarsson> i imagine working on the separate private
    >  connection is threadsafe
    > Oct 24 15:43:35 <alexlarsson> and we flush that
    > Oct 24 15:44:08 <alexlarsson> then we call dbus_bus_release_name, which i
    >  imagine would block if the dbus thread is in read_write_dispatch
    > Oct 24 15:44:35 <alexlarsson> The question is, why is the close message not
    >  delivered?
    > Oct 24 15:44:44 <alexlarsson> I mean, we flushed it...
    > Oct 24 15:45:48 <alexlarsson> sberg_: oh, i think i know
    > Oct 24 15:45:53 <alexlarsson> sberg_: release_name probably sent some message,
    >  but then the other thread pop:ed the reply
    > Oct 24 15:46:04 <alexlarsson> sberg_: queue very long wait
    > Oct 24 15:51:53 <sberg_> alexlarsson, yeah, sounds plausible; thanks!
    > Oct 24 15:52:14 <sberg_> (now that I want to reproduce it, closing succeeds
    >  cleanly every time I try, of course...)
    > Oct 24 15:54:48 <alexlarsson> sberg_: obviously
    > Oct 24 15:55:30 <alexlarsson> sberg_: that just makes the race theory more
    >  valid tho
    > Oct 24 15:57:31 <sberg_> alexlarsson, yup, after stopping the unrelated
    >  background LO build that had put the machine under full load, I see it hang
    >  again in release_name, while the execute loop is still happily running too;
    >  that nicely confirms your theory
    > Oct 24 15:58:28 <alexlarsson> sberg_: alternatively you could tell your users
    >  to always build LO in the background
    > Oct 24 15:59:04 <sberg_> I'll try to put that into the release notes; lets
    >  see...
    
    Change-Id: I2a8a58f9259d2854f42f4aa3db5bb232cf70845d
    38081c08
Adı
Son kayıt (commit)
Son güncelleme
.git-hooks Loading commit data...
UnoControls Loading commit data...
accessibility Loading commit data...
android Loading commit data...
animations Loading commit data...
apple_remote Loading commit data...
avmedia Loading commit data...
basctl Loading commit data...
basegfx Loading commit data...
basic Loading commit data...
bean Loading commit data...
bin Loading commit data...
binaryurp Loading commit data...
bridges Loading commit data...
canvas Loading commit data...
chart2 Loading commit data...
cli_ure Loading commit data...
codemaker Loading commit data...
comphelper Loading commit data...
compilerplugins Loading commit data...
config_host Loading commit data...
configmgr Loading commit data...
connectivity Loading commit data...
cppcanvas Loading commit data...
cppu Loading commit data...
cppuhelper Loading commit data...
cpputools Loading commit data...
cui Loading commit data...
dbaccess Loading commit data...
desktop Loading commit data...
dictionaries @ a79cdcb2
distro-configs Loading commit data...
drawinglayer Loading commit data...
dtrans Loading commit data...
editeng Loading commit data...
embeddedobj Loading commit data...
embedserv Loading commit data...
emfio Loading commit data...
eventattacher Loading commit data...
extensions Loading commit data...
external Loading commit data...
extras Loading commit data...
filter Loading commit data...
forms Loading commit data...
formula Loading commit data...
fpicker Loading commit data...
framework Loading commit data...
helpcompiler Loading commit data...
helpcontent2 @ 44e33e6e
hwpfilter Loading commit data...
i18nlangtag Loading commit data...
i18npool Loading commit data...
i18nutil Loading commit data...
icon-themes Loading commit data...
idl Loading commit data...
idlc Loading commit data...
include Loading commit data...
instsetoo_native Loading commit data...
io Loading commit data...
ios Loading commit data...
javaunohelper Loading commit data...
jurt Loading commit data...
jvmaccess Loading commit data...
jvmfwk Loading commit data...
l10ntools Loading commit data...
librelogo Loading commit data...
libreofficekit Loading commit data...
lingucomponent Loading commit data...
linguistic Loading commit data...
lotuswordpro Loading commit data...
m4 Loading commit data...
mysqlc Loading commit data...
nlpsolver Loading commit data...
o3tl Loading commit data...
odk Loading commit data...
offapi Loading commit data...
officecfg Loading commit data...
onlineupdate Loading commit data...
oovbaapi Loading commit data...
oox Loading commit data...
opencl Loading commit data...
osx Loading commit data...
package Loading commit data...
postprocess Loading commit data...
pyuno Loading commit data...
qadevOOo Loading commit data...
readlicense_oo Loading commit data...
registry Loading commit data...
remotebridges Loading commit data...
reportbuilder Loading commit data...
reportdesign Loading commit data...
ridljar Loading commit data...
sal Loading commit data...
salhelper Loading commit data...
sax Loading commit data...
sc Loading commit data...
scaddins Loading commit data...
sccomp Loading commit data...
schema Loading commit data...
scp2 Loading commit data...
scripting Loading commit data...
sd Loading commit data...
sdext Loading commit data...
setup_native Loading commit data...
sfx2 Loading commit data...
shell Loading commit data...
slideshow Loading commit data...
smoketest Loading commit data...
solenv Loading commit data...
soltools Loading commit data...
sot Loading commit data...
starmath Loading commit data...
stoc Loading commit data...
store Loading commit data...
svgio Loading commit data...
svl Loading commit data...
svtools Loading commit data...
svx Loading commit data...
sw Loading commit data...
swext Loading commit data...
sysui Loading commit data...
test Loading commit data...
testtools Loading commit data...
toolkit Loading commit data...
tools Loading commit data...
translations @ 0703aea9
ucb Loading commit data...
ucbhelper Loading commit data...
udkapi Loading commit data...
uitest Loading commit data...
unodevtools Loading commit data...
unoidl Loading commit data...
unoil Loading commit data...
unotest Loading commit data...
unotools Loading commit data...
unoxml Loading commit data...
ure Loading commit data...
uui Loading commit data...
vbahelper Loading commit data...
vcl Loading commit data...
winaccessibility Loading commit data...
wizards Loading commit data...
writerfilter Loading commit data...
writerperfect Loading commit data...
xmerge Loading commit data...
xmlhelp Loading commit data...
xmloff Loading commit data...
xmlreader Loading commit data...
xmlscript Loading commit data...
xmlsecurity Loading commit data...
.buckconfig Loading commit data...
.buckversion Loading commit data...
.editorconfig Loading commit data...
.gitattributes Loading commit data...
.gitignore Loading commit data...
.gitmodules Loading commit data...
.gitreview Loading commit data...
BUCK Loading commit data...
COPYING Loading commit data...
COPYING.LGPL Loading commit data...
COPYING.MPL Loading commit data...
Library_merged.mk Loading commit data...
Makefile.fetch Loading commit data...
Makefile.gbuild Loading commit data...
Makefile.in Loading commit data...
README.Solaris Loading commit data...
README.cross Loading commit data...
README.md Loading commit data...
Repository.mk Loading commit data...
RepositoryExternal.mk Loading commit data...
RepositoryFixes.mk Loading commit data...
RepositoryModule_build.mk Loading commit data...
RepositoryModule_host.mk Loading commit data...
TEMPLATE.SOURCECODE.HEADER Loading commit data...
autogen.sh Loading commit data...
config.guess Loading commit data...
config.sub Loading commit data...
config_host.mk.in Loading commit data...
config_host_lang.mk.in Loading commit data...
configure.ac Loading commit data...
download.lst Loading commit data...
g Loading commit data...
install-sh Loading commit data...
leak-suppress.txt Loading commit data...
lo.xcent.in Loading commit data...
logerrit Loading commit data...
sanitize-ubsan-blacklist Loading commit data...