[Caml-list] GC interface question

[Caml-list] GC interface question

[Dmitry Bely]

Does this fragment violate GC interface rules:

  Begin_roots_block(_varg, 2)
    _varg[0] = ((struct camlidl_intf *) this)->caml_object;
    _varg[1] = camlidl_alloc_small(1, Abstract_tag);
    Field(_varg[1], 0) = (value) log;
  End_roots();
  _vres = caml_callbackN_exn(caml_get_public_method(_varg[0],
Val_int(1007700946)), 2, _varg);

It's unsafe to pass _varg to caml_callbackN_exn when it is not
referenced from local_roots, right?

- Dmitry Bely

Re: [Caml-list] GC interface question

[Damien Doligez]

On 2011-07-01, at 11:09, Dmitry Bely wrote:

> Does this fragment violate GC interface rules:
> 
>  Begin_roots_block(_varg, 2)
>    _varg[0] = ((struct camlidl_intf *) this)->caml_object;
>    _varg[1] = camlidl_alloc_small(1, Abstract_tag);
>    Field(_varg[1], 0) = (value) log;
>  End_roots();
>  _vres = caml_callbackN_exn(caml_get_public_method(_varg[0],
> Val_int(1007700946)), 2, _varg);
> 
> It's unsafe to pass _varg to caml_callbackN_exn when it is not
> referenced from local_roots, right?


You're right, this code looks buggy.

-- Damien

Re: [Caml-list] GC interface question

[Fabrice Le Fessant]

[-- Attachment #1: Type: text/plain, Size: 636 bytes --]

I would not call camlidl_alloc_small() without first initializing
correctly _varg[1] to Val_unit.

Fabrice

On 07/01/2011 11:09 AM, Dmitry Bely wrote:
> Does this fragment violate GC interface rules:
> 
>   Begin_roots_block(_varg, 2)
>     _varg[0] = ((struct camlidl_intf *) this)->caml_object;
>     _varg[1] = camlidl_alloc_small(1, Abstract_tag);
>     Field(_varg[1], 0) = (value) log;
>   End_roots();
>   _vres = caml_callbackN_exn(caml_get_public_method(_varg[0],
> Val_int(1007700946)), 2, _varg);
> 
> It's unsafe to pass _varg to caml_callbackN_exn when it is not
> referenced from local_roots, right?
> 
> - Dmitry Bely
> 

[-- Attachment #2: fabrice_le_fessant.vcf --]
[-- Type: text/x-vcard, Size: 380 bytes --]

begin:vcard
fn:Fabrice LE FESSANT
n:LE FESSANT;Fabrice
org:INRIA Saclay -- Ile-de-France;P2P & OCaml
adr;quoted-printable:;;Parc Orsay Universit=C3=A9 ;Orsay CEDEX;;91893;France
email;internet:fabrice.le_fessant@inria.fr
title;quoted-printable:Charg=C3=A9 de Recherche
tel;work:+33 1 74 85 42 14
tel;fax:+33 1 74 85 42 49 
url:http://fabrice.lefessant.net/
version:2.1
end:vcard

Re: [Caml-list] GC interface question

[Dmitry Bely]

On Fri, Jul 1, 2011 at 3:49 PM, Damien Doligez <damien.doligez@inria.fr> wrote:
>
> On 2011-07-01, at 11:09, Dmitry Bely wrote:
>
>> Does this fragment violate GC interface rules:
>>
>>  Begin_roots_block(_varg, 2)
>>    _varg[0] = ((struct camlidl_intf *) this)->caml_object;
>>    _varg[1] = camlidl_alloc_small(1, Abstract_tag);
>>    Field(_varg[1], 0) = (value) log;
>>  End_roots();
>>  _vres = caml_callbackN_exn(caml_get_public_method(_varg[0],
>> Val_int(1007700946)), 2, _varg);
>>
>> It's unsafe to pass _varg to caml_callbackN_exn when it is not
>> referenced from local_roots, right?
>
>
> You're right, this code looks buggy.

I tend to think now that this part is OK; _varg is not used after
caml_callbackN_exn so local_roots are not needed. But what happens
inside caml_callbackN_exn()? How it assures that _varg is not
garbage-collected before the closure is applied?

This is camlidl-generated code (slightly modified to be thread-safe)
and I have an access violation (segmentation fault) error in
multi-threaded environment under heavy load. Just trying to figure out
what can be wrong here...

- Dmitry Bely

Re: [Caml-list] GC interface question

[Dmitry Bely]

On Fri, Jul 1, 2011 at 3:56 PM, Fabrice Le Fessant
<Fabrice.Le_fessant@inria.fr> wrote:
> I would not call camlidl_alloc_small() without first initializing
> correctly _varg[1] to Val_unit.

Actually, it's initialized to zero. The full generated fragment:

HRESULT STDMETHODCALLTYPE camlidl_ppm_IConfigLoad_setLogger_callback(
	struct IConfigLoad * this,
	/* in */ void *log)
{
  value _varg[2] = { 0, 0, };
  value _vres;
  HRESULT _res;
  (*camlidl_acquire_runtime)();
  Begin_roots_block(_varg, 2)
    _varg[0] = ((struct camlidl_intf *) this)->caml_object;
    _varg[1] = camlidl_alloc_small(1, Abstract_tag);
    Field(_varg[1], 0) = (value) log;
  End_roots()
  _vres = caml_callbackN_exn(caml_get_public_method(_varg[0],
Val_int(1007700946)), 2, _varg);
  if (Is_exception_result(_vres)) {
    _res = camlidl_result_exception(NULL, Extract_exception(_vres));
  } else {
    _res = S_OK;
  }
  (*camlidl_release_runtime)();
  return _res;
}

It was Xavier Leroy who initially decided to use zeroes, so I think it's OK?

- Dmitry Bely

Re: [Caml-list] GC interface question

[Fabrice Le Fessant]

[-- Attachment #1: Type: text/plain, Size: 1438 bytes --]

Initialization to 0 for _varg is OK, of course.

What does camlidl_acquire_runtime do ? I couldn't find it in camlidl
sources. Is-it possible it could trigger some garbage collection ? Is
the field this->caml_object protected from a GC move during
camlidl_acquire_runtime() ?

Fabrice

On 07/01/2011 02:54 PM, Dmitry Bely wrote:
> On Fri, Jul 1, 2011 at 3:56 PM, Fabrice Le Fessant
> <Fabrice.Le_fessant@inria.fr> wrote:
>> I would not call camlidl_alloc_small() without first initializing
>> correctly _varg[1] to Val_unit.
> 
> Actually, it's initialized to zero. The full generated fragment:
> 
> HRESULT STDMETHODCALLTYPE camlidl_ppm_IConfigLoad_setLogger_callback(
> 	struct IConfigLoad * this,
> 	/* in */ void *log)
> {
>   value _varg[2] = { 0, 0, };
>   value _vres;
>   HRESULT _res;
>   (*camlidl_acquire_runtime)();
>   Begin_roots_block(_varg, 2)
>     _varg[0] = ((struct camlidl_intf *) this)->caml_object;
>     _varg[1] = camlidl_alloc_small(1, Abstract_tag);
>     Field(_varg[1], 0) = (value) log;
>   End_roots()
>   _vres = caml_callbackN_exn(caml_get_public_method(_varg[0],
> Val_int(1007700946)), 2, _varg);
>   if (Is_exception_result(_vres)) {
>     _res = camlidl_result_exception(NULL, Extract_exception(_vres));
>   } else {
>     _res = S_OK;
>   }
>   (*camlidl_release_runtime)();
>   return _res;
> }
> 
> It was Xavier Leroy who initially decided to use zeroes, so I think it's OK?
> 
> - Dmitry Bely
> 

[-- Attachment #2: fabrice_le_fessant.vcf --]
[-- Type: text/x-vcard, Size: 380 bytes --]

begin:vcard
fn:Fabrice LE FESSANT
n:LE FESSANT;Fabrice
org:INRIA Saclay -- Ile-de-France;P2P & OCaml
adr;quoted-printable:;;Parc Orsay Universit=C3=A9 ;Orsay CEDEX;;91893;France
email;internet:fabrice.le_fessant@inria.fr
title;quoted-printable:Charg=C3=A9 de Recherche
tel;work:+33 1 74 85 42 14
tel;fax:+33 1 74 85 42 49 
url:http://fabrice.lefessant.net/
version:2.1
end:vcard

Re: [Caml-list] GC interface question

[Dmitry Bely]

On Fri, Jul 1, 2011 at 5:37 PM, Fabrice Le Fessant
<Fabrice.Le_fessant@inria.fr> wrote:
> Initialization to 0 for _varg is OK, of course.
>
> What does camlidl_acquire_runtime do ? I couldn't find it in camlidl
> sources.

Yes, that's my modification. Original camlidl is not thread-safe.
Basically camlidl_acquire_runtime does caml_c_thread_register and
caml_leave_blocking_section. camlidl_release_runtime points to
caml_enter_blocking_section.

> Is it possible it could trigger some garbage collection ?

AFAIK, caml_leave_blocking_section() cannot trigger GC (while
caml_enter_blocking_section can).

> Is the field this->caml_object protected from a GC move during
> camlidl_acquire_runtime() ?

Yes, by caml_register_global_root.

Returning to my question:

value _varg[2] = { 0, 0, };

Is such initialization with zero normal?

- Dmitry Bely

Re: [Caml-list] GC interface question

[Fabrice Le Fessant]

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

On 07/01/2011 04:45 PM, Dmitry Bely wrote:
>> Initialization to 0 for _varg is OK, of course.

Is it the reply to your former question ?

> Yes, that's my modification. Original camlidl is not thread-safe.
> Basically camlidl_acquire_runtime does caml_c_thread_register and
> caml_leave_blocking_section. camlidl_release_runtime points to
> caml_enter_blocking_section.

If I understand your code, you release the master lock on the GC, and
then you call camlidl_alloc_small(). Shouldn't you hold the master lock
while allocating ?

Fabrice

[-- Attachment #2: fabrice_le_fessant.vcf --]
[-- Type: text/x-vcard, Size: 380 bytes --]

begin:vcard
fn:Fabrice LE FESSANT
n:LE FESSANT;Fabrice
org:INRIA Saclay -- Ile-de-France;P2P & OCaml
adr;quoted-printable:;;Parc Orsay Universit=C3=A9 ;Orsay CEDEX;;91893;France
email;internet:fabrice.le_fessant@inria.fr
title;quoted-printable:Charg=C3=A9 de Recherche
tel;work:+33 1 74 85 42 14
tel;fax:+33 1 74 85 42 49 
url:http://fabrice.lefessant.net/
version:2.1
end:vcard

Re: [Caml-list] GC interface question

[Dmitry Bely]

On Fri, Jul 1, 2011 at 7:09 PM, Fabrice Le Fessant
<Fabrice.Le_fessant@inria.fr> wrote:
> On 07/01/2011 04:45 PM, Dmitry Bely wrote:
>>> Initialization to 0 for _varg is OK, of course.
>
> Is it the reply to your former question ?

Yes, thanks.

>> Yes, that's my modification. Original camlidl is not thread-safe.
>> Basically camlidl_acquire_runtime does caml_c_thread_register and
>> caml_leave_blocking_section. camlidl_release_runtime points to
>> caml_enter_blocking_section.
>
> If I understand your code, you release the master lock on the GC,

No.

> and then you call camlidl_alloc_small(). Shouldn't you hold the master lock
> while allocating ?

Of course. This function is called from C when no Ocaml code is
running and the Ocaml master lock is released. So I acquire it on
enter  (camlidl_acquire_runtime), call an Ocaml callback and release
on return (camlidl_release_runtime).

- Dmitry Bely