[Libreoffice] Multiple issues with cppuhelper/source/exc_thrower.cxx

[Caolán McNamara]

On Fri, 2011-05-27 at 10:53 +0200, Bjoern Michaelsen wrote: 
> Hi Caolán, all,
> 
> Digging deeper, I found ExceptionThrower::get() on the 3.4 branch to:
> a) not using a static variable for s_pThrower thus making the if always
>    eval to true.

heh!

> b) not even double-checking s_pThrower after aquireing the guard, making
>    the whole thing pretty pointless.
> c) However, I think even as is, the should not be harmful at least on
>    gcc, if http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13684 is really
>    fixed (and the testcode in the issue performs good on my machine).
>    But what about other compilers?

Well, if we look at the original code, given that s_pThrower is just a
normal local variable set to 0, then we *always* enter the first if, and
the first if always takes a MutexGuard from the global mutex, so we
always lock inside the first branch, which is the only one executed.

So the original code doesn't do what it apparently thought it was doing
of using a double-check lock, but ends up using a unconditional mutex
lock every time, so it should be ok on all platforms/compilers, no ?

> Caolan has fixed this on master with commit
> ure:c51c13ff92adbe1d3f22bee6d907132c48d16602, if it was broken.
> Depending on how compilers handle this, it might be worth to cherrypick
> to 3-4.

I think, given the unconditional mutex lock that it should be ok as it
stands for 3-4.

> So unless I overlooked something there, the error really is not in the
> singleton creation (at least with my gcc)

That seems to be the case. The bug report's stack is from svx
autocorrect, which means that ucbhelper::cancelCommandExecution and
cppu::throwException have successfully thrown exceptions at least a
hundred times or so before the crash, so its not the case that it's e.g.
the first throw or two through the uno bridge.

> leading to the cause to
> possibly be something going wrong in the uno2cpp.mapInterface(..) call
> just before. Does that sound plausible? I just want to make sure, before
> digging even deeper into the disassembly.

Is the crash reproducible for you in any way ? It'd be much more
confident with a reproducible scenario we could run under valgrind.

As an aside, what gcc is in use ? I recall a gcc bug where in a tight
loop each exception thrown incrementally busted unwind info 16bytes at a
time

https://bugzilla.redhat.com/show_bug.cgi?id=447912
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36419

but that's an old long-fixed bug now.

C.