Re: BonoboUnknown & G_SIGNAL_TYPE_STATIC_SCOPE

[Owen Taylor <otaylor redhat com>]

Michael Meeks <michael ximian com> writes:

> Hi Owen,
> 
> On Wed, 2002-01-02 at 20:41, Owen Taylor wrote:
> > I have to agree with Havoc that this is an inappropriate usage. 
> > You are only allowed to use G_SIGNAL_TYPE_STATIC_SCOPE for
> > signal arguments, so including it in the type macro is going
> > to cause people very unexpected problems.
> 
> 	I agree it's ugly; but the problem is that if it is included
> erroneously, it seems people get g_asserts; if it is not, people get
> performance problems that they don't understand - I'd prefer the former
> - or to totaly kill bonobo-type.[ch] and use untyped pointers for the
> lot really [ which doesn't help bindings much ].

There is a contract in the way that GObject works in that if you
provide a boxed type (or anything but a G_TYPE_POINTER) as an argument
to a signal, it will persist until the emission is over unless you |
in G_SIGNAL_TYPE_STATIC_SCOPE. If you stick the G_SIGNAL_TYPE_STATIC_SCOPE
into BONOBO_TYPE_UNKNOWN you are violating this contract, not
to mention g_assert() problems and the fact that you are forcing
people to use get_type() in some cases.

People just have to learn to always use G_SIGNAL_TYPE_STATIC_SCOPE;
doing it magically for one type doesn't help.
 
> > Sheesh, can't we add a bit of magic / evil C API so that Bonobo_Unknown_ref()
> > only holds one reference to the remote object for all local references?
> > This doesn't seem like the only place people are going to get bitten.
> 
> 	No - sadly not; the way the reference counting convention was
> constructed does not allow us to distinguish when a Bonobo reference is
> being handed, and when a 'weak' or CORBA only reference is being passed.
> Thus it is not possible to reliably cache the Bonobo_Unknown reference
> count - sad but true.

Hmm, yeah, forgot about that problem. 

This particular problem could still be helped with some Bonobo-internal
magic ... since we can't transfer ownership of a boxed object to
a different process we could keep locally a count of "non-transferrable"
reference counts that proxy to a single remote reference count
and use that for the copy/free functions for BONOBO_TYPE_OBJECT.

It would probably have to be kept strictly internal, since the
exposure of that concept along with the presence of COBRA_Object_duplicate
would most likely terminally confuse everyone.

> 	Of course; we could add API to make this distinction I suppose - not
> too tough; but it seems better to me to wait for Mono & a garbage
> collected world, to solve some of the problems at root. Of course -
> making the above distinction would be somewhat extremely useful for
> other purposes as well, such as connection based reference accounting
> etc. but sadly - it's really not feasible to change at this stage IMHO. 

The problem isn't really the lack of local garbage collection
here, it's the whole problem of pretending we can GC remote
objects in the same way as local objects. But yes, that's not really 
fixable in the scope of Bonobo.
 
> 	Either way - I remain interested as to the possible merit of the
> default behavior copying all the effectively 'const in' arguments to
> these signals - for what is this useful ?. 

The idea is, is that without G_SIGNAL_TYPE_STATIC_SCOPE I can write:

gtk_style_set_font (GtkStyle *style, GdkFont *new_font)
{
  g_signal_emit_by_name (style, "about-to-set-font",
                         style->old_font, new_font);

  style->font = g_object_ref (new_font);
  g_object_unref (old_font);
                            
}

Which would be improper normally since if set_font() was
called again, the old_font parameter would be come invalid.
It's adding some automatic protection against reentrancy
problem.

The idea is that before you add STATIC_SCOPE you need to
analyze all your emit functions and make sure the parameter
is not going be freed for the duration of the emission.

Yes, it's making things more complicated for the sake of making
them "easier"; I fought this for a few months, but I had to
go on to other things eventually.

> Similarly I'm intrigued by the amount of locking going on in
> gsignal.c - as I'm given to understand that signal emission is not
> thread safe - [ is this the case ? ].

No signal emission on a single object from multiple threads is not
fully thread safe (though looking at gsignal.c it seems a lot closer
than I thought), but:

  - GSignal stores the signal handlers in a global hash by
    instance location so you can have signals on non-GObjects.
    (Yep, though they have to have a destructor to call
    g_signal_handlers_destroy(), so not GdkRectangle...)
  
  - Just as we used to have problem with GType being an integer,
    we have problems with signal ids being integers.

  - Some parts of the SignalNode structure are mutable, such
    as the list of emission hooks.

Regards,
                                        Owen