[omniORB] Deadlock on recursive invocation with SINGLE_THREAD_MODEL

[Felix Nawothnig]

Hello.

Looks like when doing an recursive invokation on a POA with the
SINGLE_THREAD_MODEL policy (with the outer request obviously being
performed in it's main-thread) omniORB deadlocks.

The reasons are obvious, and it might not be an actual bug (I really don't
care what the specs say about this, if anything) but as the ORB could -
theoretically - recursively dispatch incoming requests while waiting for
an outgoing one to finish I'm wondering what the developers think about
this...

Cheers,

Felix

[Michael]

omniORB behaves correctly.
...and the developers think: Avoid SINGLE_THREAD_MODEL, use locks to
serialize your objects.

:)

Post by Felix Nawothnig
Hello.
Looks like when doing an recursive invokation on a POA with the
SINGLE_THREAD_MODEL policy (with the outer request obviously being
performed in it's main-thread) omniORB deadlocks.
The reasons are obvious, and it might not be an actual bug (I really don't
care what the specs say about this, if anything) but as the ORB could -
theoretically - recursively dispatch incoming requests while waiting for
an outgoing one to finish I'm wondering what the developers think about
this...
Cheers,
Felix
_______________________________________________
omniORB-list mailing list
http://www.omniorb-support.com/mailman/listinfo/omniorb-list

[Felix Nawothnig]

Post by Michael
omniORB behaves correctly.

Well, I didn't really claim otherwise... but given the choice between an
operation deadlocking and not deadlocking I don't quite see the benefit of
the former.

Post by Michael
...and the developers think: Avoid SINGLE_THREAD_MODEL, use locks to
serialize your objects.

When dealing with inherently single-threaded software components that's
hardly sensible. Ignoring the obviously unnecessary boilerplate code (and
lots of places to forget the lock...) for the sake of argument - putting
one giant lock around everything doesn't help avoiding the
recursive-invokation deadlock at all...

Yes, one could use a recursive mutex... not very nice though.

And all that doesn't help when you're dealing with software which expects
(and verifies) that it's being accessed from a single thread. Note that
I'm using such software - and it's not some obscure arcane proprietary
crap but just Qt.

Now you need to marshal every request, along with it's parameters into
that thread and back. That will obviously result in about the same amount
of hand-written boilerplate code as generated by the IDL compiler, after
all you are doing nearly the same thing.

I worked on a project where developers did exactly that (for no apparent
reason though, in their case there was no thread-safety needed .. nor
gained).

About a year after I ripped out the code in question (about 50k lines of
fabulous copy&paste-work) I can laugh about it, but when you have to deal
with code like that it hurts.

Badly.

So _please_ don't tell me do this. ;-)

So, no, SINGLE_THREAD_MODEL does not need to be feared. It has some real
world uses. And my book says this is _exactly_ what it is there for.

Cheers,

Felix

[Michael]

Post by Felix Nawothnig

Post by Michael
omniORB behaves correctly.

Well, I didn't really claim otherwise... but given the choice between an
operation deadlocking and not deadlocking I don't quite see the benefit of
the former.

Post by Michael
...and the developers think: Avoid SINGLE_THREAD_MODEL, use locks to
serialize your objects.

When dealing with inherently single-threaded software components that's
hardly sensible. Ignoring the obviously unnecessary boilerplate code (and
lots of places to forget the lock...) for the sake of argument - putting
one giant lock around everything doesn't help avoiding the
recursive-invokation deadlock at all...
Yes, one could use a recursive mutex... not very nice though.
And all that doesn't help when you're dealing with software which expects
(and verifies) that it's being accessed from a single thread. Note that
I'm using such software - and it's not some obscure arcane proprietary
crap but just Qt.
Now you need to marshal every request, along with it's parameters into
that thread and back. That will obviously result in about the same amount
of hand-written boilerplate code as generated by the IDL compiler, after
all you are doing nearly the same thing.
I worked on a project where developers did exactly that (for no apparent
reason though, in their case there was no thread-safety needed .. nor
gained).
About a year after I ripped out the code in question (about 50k lines of
fabulous copy&paste-work) I can laugh about it, but when you have to deal
with code like that it hurts.
Badly.
So _please_ don't tell me do this. ;-)
So, no, SINGLE_THREAD_MODEL does not need to be feared. It has some real
world uses. And my book says this is _exactly_ what it is there for.
Cheers,
Felix

No doubt there has been a reason why this policy is in the standard. All
I wanted to say is that usually it causes lots of trouble (which is not
CORBA related but just based on the fact that it is single threaded). If
I remember correctly omniORB implements single_threaded through locking
anyway (but maybe Duncan would be more competent to answer that).

You could also implent locking in interceptors, much less copy and paste
in that case.

And yes, sometimes things are painful :)

[Duncan Grisby]

Post by Felix Nawothnig
Looks like when doing an recursive invokation on a POA with the
SINGLE_THREAD_MODEL policy (with the outer request obviously being
performed in it's main-thread) omniORB deadlocks.

What exactly are you doing? The SINGLE_THREAD_MODEL does in fact use a
recursive mutex, as required by the POA specification. If you just
recursively call operations on the same POA, from the same thread, it
will work ok.

Post by Felix Nawothnig
The reasons are obvious, and it might not be an actual bug (I really don't
care what the specs say about this, if anything) but as the ORB could -
theoretically - recursively dispatch incoming requests while waiting for
an outgoing one to finish I'm wondering what the developers think about
this...

It sounds like you are actually doing a remote call from inside your
servant, and the remote side calls back? That definitely can't work.
There's no way to know that any incoming calls are from the same logical
thread of control that was holding the lock. It certainly isn't
acceptable to allow any incoming call, since that would violate the
single threading requirement.

Cheers,

Duncan.

[Felix Nawothnig]

Post by Duncan Grisby

Post by Felix Nawothnig
The reasons are obvious, and it might not be an actual bug (I really
don't care what the specs say about this, if anything) but as the ORB
could - theoretically - recursively dispatch incoming requests while
waiting for an outgoing one to finish I'm wondering what the
developers think about this...

It sounds like you are actually doing a remote call from inside your
servant, and the remote side calls back?

That's what I meant, yes.

Post by Duncan Grisby
That definitely can't work. There's no way to know that any incoming
calls are from the same logical thread of control that was holding the
lock. It certainly isn't acceptable to allow any incoming call, since
that would violate the single threading requirement.

Well, it would be enough to just do it for objects served by POAs with
SINGLE_THREAD_MODEL, but I realize that it might not be okay if a
completely unrelated request (on a completely unrelated object served by
the same POA maybe) changes the servant world in-between an outgoing
invokation...

Makes me wish CORBA knew const methods, which could be declared to be
permitted to be called in that case.

My concrete example is:

interface IObserver { string getName(); }
interface ISubject { void registerObserver(IObserver observer); }

My application calls subject->registerObserver(_this()), the subject asks
the observer about it's name and adds it to it's registry (for logging
purposes). Now, it doesn't have to work recursively (i have decoupled it
now), but I kinda liked the idea that the subject first queries the
observer about details before returning from the call.

In another project (where I don't use SINGLE_THREAD_MODEL) I use the same
pattern to do version-checks and the like upon remote registration - and I
couldn't decouple it there (as in case of failure it throws an exception).

Obviously you could always pass the data in the initial registration...
but that'd kinda defeat the purpose, as it's no longer a real
verification.

And that kind of verification does actually serve a few uses (although the
servant can always lie)... for example: In my getName() implementation I
use RTTI to retrieve the actual servant class - if the locator dispatches
to the wrong servant the other side will notice.

Well. I'm just trying to explain that there is a valid reason why I would
want this.

But I understand why it's not being done. :-)

Cheers,

Felix

[Duncan Grisby]

On Friday 11 September, "Felix Nawothnig" wrote:

[...]

Post by Felix Nawothnig
Well, it would be enough to just do it for objects served by POAs with
SINGLE_THREAD_MODEL, but I realize that it might not be okay if a
completely unrelated request (on a completely unrelated object served by
the same POA maybe) changes the servant world in-between an outgoing
invokation...

The POA spec says this about SINGLE_THREAD_MODEL (section 11.3.7.1 of
the CORBA 2.6 spec):

SINGLE_THREAD_MODEL - Requests for a single-threaded POA are
processed sequentially. In a multi-threaded environment, all upcalls
made by this POA to implementation code (servants and servant
managers) are made in a manner that is safe for code that is
multi-thread-unaware. The POA will still allow reentrant calls from
an object implementation to itself, or to another object
implementation managed by the same POA.

The only way to make it "safe for code that is multi-thread-unaware" is
to use a lock. Allowing re-entrant calls when the thread happened to be
making an outgoing CORBA call would, as you say, not protect the
multi-thread-unaware code sufficiently.

Cheers,

Duncan.