add caml_weak_get(), caml_weak_set() and caml_weak_check() to caml/*.h

[vicuna]

Original bug ID: 7173
Reporter: goswin
Status: closed (set by @mshinwell on 2016-12-08T13:31:18Z)
Resolution: duplicate
Priority: normal
Severity: feature
Version: 4.02.3
Category: runtime system and C interface
Monitored by: @gasche bobot @hcarty

Bug description

At the moment it is impossible to access a Weak.t from a C stub in a proper manner because the memory layout of Weak.t is undocumented and there are not accessor functions being exported. Current behaviour even departs from what is documented in the wild.

Complicating things it seems that with 4.03 the memory layout of Weak.t changes, moving all items down one index to make room for the ephemeral field or something. Every C stub using a Weak.t will have to be fixed. Now would be the right time to provide accessor functions to people can access Weak.t properly in the future.

[vicuna]

Comment author: @mshinwell

If 4.03 has changed the memory layout of the weak blocks, this should be documented somewhere. Is it?

[vicuna]

Comment author: bobot

I think it is not. Since weak.h is not in the public include and nthing is said about the layout of weak blocks in the documentation I though nobody used weak pointer from C.

In 4.03 weak.h is still not public but define the following constant:

extern value caml_ephe_none;


/** The first field 0:  weak list;
       second field 1:  data;
       others       2..:  keys;
    A weak pointer is an ephemeron with the data at caml_ephe_none
    If fields are added, don't forget to update weak.ml [additional_values].
 */

#define CAML_EPHE_LINK_OFFSET 0
#define CAML_EPHE_DATA_OFFSET 1
#define CAML_EPHE_FIRST_KEY 2

This interface is too basic, and really not for the public, but I think we don't want either to give access to caml_weak_get() for example which makes an allocation (option type). Something that return the null pointer in the case None seems more C friendly.

In 4.03 a bug (it is in the changes) when two weak arrays contains the same value have been fixed. It make caml_weak_get a little more complicated because it is not sufficient anymore to test if the fields is caml_ephe_none or not.

To sum-up, I think weak have never been given a real C API. Still something should be added to the changes at least for politeness about the change in layout and access. Since now accessing weak pointers (not due to ephemerons but due to the fix for weak) is a little more complicated, people should not try to access them themselves. So the question, should we try to give a C interface for weak/ephemerons this late for 4.03 since people are using weak from C? Or could we wait for 4.04? In anycase I'm going to try to make a PR for a tentative C API.

[vicuna]

Comment author: @mshinwell

I would suggest that for 4.03 we just document the change.

[vicuna]

Comment author: @stedolan

I would prefer to avoid documenting the memory layout for weak pointers, since it's going to change again in multicore. A C API to access weak pointers sounds like a good idea, though.

[vicuna]

Comment author: bobot

Oh, really? I understood there was nothing problematic with weak pointers for multicore. Though I added for the fix for weak pointers a new phase Clean between mark and sweep. And I don't know if you want to iterate concurrently on the list of ephemerons which is dynamically reordered. So indeed it is not so simple.

[vicuna]

Comment author: goswin

I'm not sure how everybody else has handled dynamic C structures with closures in harmony with the Gc but Weak.t is the only way I see to make that work so memory isn't leaked. So some form of interface is needed. A non-allocating one would be preferable. Should be save to assume nobody stores "Some nullptr" in a Weak.t.

While you change Weak.t it would be real nice if the Weak.t still contained the value while the finaliser for that value runs. After all the code is allowed to store the value somewhere to make it alive again. Makes no sense to me that the Weak.t is (temporarily?) empty when the object then remains alive in the end.

[vicuna]

Comment author: @mshinwell

I don't think this needs to be classified as urgent, although it needs addressing.

[vicuna]

Comment author: bobot

While you change Weak.t it would be real nice if the Weak.t still contained the value while the finaliser for that value runs. After all the code is allowed to store the value somewhere to make it alive again. Makes no sense to me that the Weak.t is (temporarily?) empty when the object then remains alive in the end.

If it was the case, it is not the case anymore in 4.03, the addition of the clean phase makes the semantic of weak pointer more straightforward.

[vicuna]

Comment author: goswin

I marked it as urgent because 4.03 will break code where people have figured out how to access a Weak.t like me. Just like they figured out how to access a 'a option or a reference.

[vicuna]

Comment author: bobot

Just like they figured out how to access a 'a option or a reference.

I disagree, it is not the same thing. In 4.02 ocaml manual chapter "19.3 Representation of OCaml data types", the layouts of structure and algebraic datatypes are described. It is not the case for Weak.t.

[vicuna]

Comment author: @gasche

I think that's a rather fine line -- I mean I think that both bobot and goswin are right in this case. Whether or not it was reasonable to exploit Weak.t's representation in the first place (bobot is right that it was not), the idea of proposing a better interface at the same time that the representation changes is good.

François, would you be able to write a library implementing this interface without too much work? If so, then it becomes a question of whether we have the "change budget" to have it in 4.03. My gut feeling is that we haven't (it's too late), but couldn't we try to distribute the library as a separate library during the 4.03 release? Is it easy for people to rely on C libraries distributed through ocamlfind? If so, I think it's worth trying to do it.

[vicuna]

Comment author: bobot

Gasche, it is a nice idea to target the library for 4.04 and when it is merged in trunk propose a C library through opam that implement the API.

I started, the first part is to rename weak.h to weak_private.h and to add a new weak.h public. I think I will implement all directly in weak.h for simplicity in the compilation of the library ;)

[vicuna]

Comment author: @gasche

My idea was to propose the interface as an external library that would be available for <4.03 versions (legacy interface), =4.03 version (new interface), and >4.03 (shim library if this becomes available upstream, or nothing at all if .h include paths work this way). But you know better about this stuff.

[vicuna]

Comment author: @jhjourdan

I really don't understand why this would be impossible to put this library in the 4.03 release. This is really no more than a few dozen LOC, all the technical meat is already written for caml_weak_set/get...

[vicuna]

Comment author: @gasche

Well, you should just ask Damien? Also, if either you or François could actually write this dozen of lines, it would be a good basis for further discussion. (If you wait for a decision it may be too late.)