Re: Lisp object that refers to a C struct

[Eli Zaretskii <eliz@gnu.org>]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: lekktu@gmail.com, eggert@cs.ucla.edu, emacs-devel@gnu.org
> Date: Fri, 19 Oct 2012 10:44:32 -0400
> 
> >> > It doesn't.  I meant the need to manage the table itself, grow it when
> >> > needed, etc.
> >> To me "table" doesn't imply "array".  It's just some kind of
> >> data-structure that keeps the elements at hand.  It can be a list, an
> >> array, a tree, a has-table, you name it.
> > If we use a Lisp data structure, then the same issue of putting a bare
> > pointer into that started this thread it pops up again, doesn't it?
> > Anyway, a Lisp data structure is what I have now.
> 
> In any case, the code now does keep a table of those C structs, and they
> are not reclaimed when the Lisp code loses the last "handle".
> Instead they are only reclaimed when the Lisp code calls
> remove-watch explicitly.

That's true.  The reason is that the C struct cannot be reclaimed as
long as the watcher thread runs, because that thread constantly
dereferences a pointer to the struct.  The remove-watch API takes care
of orderly shutting down the thread (which is not trivial, because it
is mostly stuck in a system call), and then frees the struct.

> This design means we don't need a "finalizer" (i.e. code in gc_sweep
> that calls us back to free the C struct), but it also means that we may
> "leak" those C structs if the Lisp code just forgets to `remove-watch'.

Correct.

> I think that's OK for now.

I concur.

> >> > Call w32_valid_pointer_p, and in addition verify that the struct
> >> > pointed to by it has the correct magic signature.
> >> Why is that needed?
> > To make sure we never dereference a pointer that doesn't point to the
> > watch object.  Since the remove-watch API accepts a Lisp integer, it
> > could be called with any arbitrary integer value.
> 
> But as long as the table is not corrupted, there's no risk of such
> a thing happening anyway.

Right.

> > IOW, I don't want to crash, even if somehow a bad pointer is found in
> > the alist described below.
> 
> OK, so it's just done out of paranoia.  That's fine, but please make it
> clear in the code, e.g. by placing it in an eassert.

Will do.

> > As I wrote, the reason for this design of the alist was to be 100%
> > compatible with what Rüdiger did.  Otherwise, I could keep CALLBACK in
> > the C struct as well, for example.
> 
> I think it's OK.  Encoding C pointers in Lisp integers is pretty ugly,
> so we should make sure that no part of the design depends on it (so it
> can be changed in the future)

Well, the way to get the pointer to the struct given the descriptor
and vice versa _will_ have to change if the design changes.  Other
than that, nothing depends on that.

> and we should also make it very clear in the doc that Elisp code
> should not rely on those handles being integers (treat them as black
> boxes instead).

They are documented as "descriptors".  I will see if there's a place
to make the point you want to drive home.

Anyway, I guess I'm waiting for Rüdiger to install his changes, is
that right?