Module support: No environment in pointer release function

Module support: No environment in pointer release function

[Elias Mårtenson]

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

I'm currently building an Emacs module to provide access to GSSAPI. In it,
I have to use the make_user_ptr() function in a few places. When creating a
user_ptr, the function accepts a pointer to a function which will be called
when the object is GC'ed.

The problem is that the free function is only called with a single
argument; the pointer itself. In my case, the underlying GSSAPI function
that releases the object may return some diagnostics, and I would like to
be able to call the Elisp function ‘warn’ with a message describing what
happened.

I think that the callback should accept 3 arguments instead of 1. In
addition to the pointer itself, the ‘emacs_env’ pointer is needed. An
arbitrary ‘void *data’ pointer would be useful too.

Regards,
Elias

[-- Attachment #2: Type: text/html, Size: 883 bytes --]

Re: Module support: No environment in pointer release function

[Paul Eggert]

Elias Mårtenson wrote:
> In
> addition to the pointer itself, the ‘emacs_env’ pointer is needed. An
> arbitrary ‘void *data’ pointer would be useful too.

Can you package up these pointers into a single data structure, and pass a 
pointer to that structure? That's what's usually done in situations like this, 
and is why a single 'void *' should suffice.

Re: Module support: No environment in pointer release function

[Elias Mårtenson]

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

On 6 February 2017 at 13:33, Paul Eggert <eggert@cs.ucla.edu> wrote:

> Elias Mårtenson wrote:
>
>> In
>> addition to the pointer itself, the ‘emacs_env’ pointer is needed. An
>> arbitrary ‘void *data’ pointer would be useful too.
>>
>
> Can you package up these pointers into a single data structure, and pass a
> pointer to that structure? That's what's usually done in situations like
> this, and is why a single 'void *' should suffice.
>

I could do this, but as far as I understood, the emacs_env pointer is not
guaranteed to be immutable. If it isn't immutable, then I might just as
well save it to a global variable and not have to pass it through every
function that needs it.

What are the guarantees with regards to the emacs_env value?

Regards,
Elias

[-- Attachment #2: Type: text/html, Size: 1448 bytes --]

Re: Module support: No environment in pointer release function

[Paul Eggert]

Elias Mårtenson wrote:
> What are the guarantees with regards to the emacs_env value?

Sorry, I'm the wrong guy to ask.

Re: Module support: No environment in pointer release function

[Philipp Stephani]

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

Elias Mårtenson <lokedhs@gmail.com> schrieb am Di., 7. Feb. 2017 um
04:16 Uhr:

> On 6 February 2017 at 13:33, Paul Eggert <eggert@cs.ucla.edu> wrote:
>
> Elias Mårtenson wrote:
>
> In
> addition to the pointer itself, the ‘emacs_env’ pointer is needed. An
> arbitrary ‘void *data’ pointer would be useful too.
>
>
> Can you package up these pointers into a single data structure, and pass a
> pointer to that structure? That's what's usually done in situations like
> this, and is why a single 'void *' should suffice.
>
>
> I could do this, but as far as I understood, the emacs_env pointer is not
> guaranteed to be immutable. If it isn't immutable, then I might just as
> well save it to a global variable and not have to pass it through every
> function that needs it.
>
> What are the guarantees with regards to the emacs_env value?
>

The big restriction is that its lifetime is limited to the lifetime
(storage duration) of the argument that is used to obtain the pointer. I.e.
in a function

emacs_value module_fun(emacs_env* env, ...) { ... }

the environment represented by env is only valud in the body of module_fun;
reusing env or *env once the body is finished is undefined behavior.
(I'm aware that this is not documented at all, and I've been meaning to
write some documentation for a while.)

[-- Attachment #2: Type: text/html, Size: 2566 bytes --]

Re: Module support: No environment in pointer release function

[Philipp Stephani]

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

Paul Eggert <eggert@cs.ucla.edu> schrieb am Mo., 6. Feb. 2017 um 06:34 Uhr:

> Elias Mårtenson wrote:
> > In
> > addition to the pointer itself, the ‘emacs_env’ pointer is needed. An
> > arbitrary ‘void *data’ pointer would be useful too.
>
> Can you package up these pointers into a single data structure, and pass a
> pointer to that structure? That's what's usually done in situations like
> this,
> and is why a single 'void *' should suffice.
>
>
That doesn't work because the module environment is only valid during a
single module function call and can't be reused.
(With threads, it's also bound to the thread it was created in, and I don't
think the GC will run in the same thread.)

[-- Attachment #2: Type: text/html, Size: 1211 bytes --]

Re: Module support: No environment in pointer release function

[Philipp Stephani]

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

Elias Mårtenson <lokedhs@gmail.com> schrieb am Fr., 3. Feb. 2017 um
03:56 Uhr:

> I'm currently building an Emacs module to provide access to GSSAPI. In it,
> I have to use the make_user_ptr() function in a few places. When creating a
> user_ptr, the function accepts a pointer to a function which will be called
> when the object is GC'ed.
>
> The problem is that the free function is only called with a single
> argument; the pointer itself. In my case, the underlying GSSAPI function
> that releases the object may return some diagnostics, and I would like to
> be able to call the Elisp function ‘warn’ with a message describing what
> happened.
>
> I think that the callback should accept 3 arguments instead of 1. In
> addition to the pointer itself, the ‘emacs_env’ pointer is needed. An
> arbitrary ‘void *data’ pointer would be useful too.
>

This would require two things:
- It would need to be possible to run arbitrary Lisp code during GC. I
don't think that's the case right now.
- It would require a second type of user pointer because the existing one
is already published.

The second requirement wouldn't be a big deal from a technical perspective,
but would make the interface more complex, and I don't know whether it's a
good tradeoff just to show a warning. The first requirement would likely be
a deal-breaker.

[-- Attachment #2: Type: text/html, Size: 1854 bytes --]

Re: Module support: No environment in pointer release function

[Andreas Politz]

Elias Mårtenson <lokedhs@gmail.com> writes:

> [...] In it, I have to use the make_user_ptr() function in a few
> places. When creating a user_ptr, the function accepts a pointer to a
> function which will be called when the object is GC'ed.

How do you even know its your pointer and not someone elses ?

-ap

Re: Module support: No environment in pointer release function

[Elias Mårtenson]

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

On 9 February 2017 at 01:03, Andreas Politz <politza@hochschule-trier.de>
wrote:

> Elias Mårtenson <lokedhs@gmail.com> writes:
>
> > [...] In it, I have to use the make_user_ptr() function in a few
> > places. When creating a user_ptr, the function accepts a pointer to a
> > function which will be called when the object is GC'ed.
>
> How do you even know its your pointer and not someone elses ?


Because it was me who created the pointer. In the specific case, when I
call the GSSAPI function gss_import_name(), this function returns a pointer
to an opaque object which needs to be released using the function
gss_release_name().

I package up this pointer using env->make_user_pointer() which accept a
function that is responsible for freeing the pointer when it's GC'ed.

The issue is that gss_import_name() can return an error. Of course, I've
never actually seen it return an error, and I doubt that it ever will in
practice. But I'm struggling with the correct way to handle an error. At
the very least, I want to know it happened since that would reveal a bug in
my code.

Right now I simply call abort() if there is an error. Excessive, I know,
but at least I know when there is a problem and the stack trace gives me
all the answers I need.

I'm not sure I should keep the abort() in the final version though.

Regards,
Elias

[-- Attachment #2: Type: text/html, Size: 1909 bytes --]

Re: Module support: No environment in pointer release function

[Elias Mårtenson]

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

On 10 February 2017 at 13:21, Elias Mårtenson <lokedhs@gmail.com> wrote:

The issue is that gss_import_name() can return an error.
>

Sorry for replying to myself, but the above must seem very confusing for
someone trying to read it.

I meant to say gss_release_name() in the quote above.

Regards,
Elias

[-- Attachment #2: Type: text/html, Size: 798 bytes --]

Re: Module support: No environment in pointer release function

[Andreas Politz]

Elias Mårtenson <lokedhs@gmail.com> writes:

>  How do you even know its your pointer and not someone elses ?

> Because it was me who created the pointer.

Sorry for being so terse.  This is not exactly related to your problem,
but a general idea:  What happens if multiple modules use user_ptr and
the user takes one from module A and passes it to module B ?

Anyway, I think what you could do right now is save the result of your
library function inside your module and retrieve it with a finalizer
(see make-finalizer).

-ap

Re: Module support: No environment in pointer release function

[Philipp Stephani]

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

Elias Mårtenson <lokedhs@gmail.com> schrieb am Fr., 10. Feb. 2017 um
06:21 Uhr:

> On 9 February 2017 at 01:03, Andreas Politz <politza@hochschule-trier.de>
> wrote:
>
> Elias Mårtenson <lokedhs@gmail.com> writes:
>
> > [...] In it, I have to use the make_user_ptr() function in a few
> > places. When creating a user_ptr, the function accepts a pointer to a
> > function which will be called when the object is GC'ed.
>
> How do you even know its your pointer and not someone elses ?
>
>
> Because it was me who created the pointer. In the specific case, when I
> call the GSSAPI function gss_import_name(), this function returns a pointer
> to an opaque object which needs to be released using the function
> gss_release_name().
>
> I package up this pointer using env->make_user_pointer() which accept a
> function that is responsible for freeing the pointer when it's GC'ed.
>
> The issue is that gss_import_name() can return an error. Of course, I've
> never actually seen it return an error, and I doubt that it ever will in
> practice. But I'm struggling with the correct way to handle an error. At
> the very least, I want to know it happened since that would reveal a bug in
> my code.
>
> Right now I simply call abort() if there is an error. Excessive, I know,
> but at least I know when there is a problem and the stack trace gives me
> all the answers I need.
>
> I'm not sure I should keep the abort() in the final version though.
>

Probably not. You should either log (on stderr) and ignore the error, or
not call gss_release_name (or any other function that can fail) in a
finalizer.
Just like they Java counterpart, finalizers can't fail. There's also no
guarantee that they ever be called, or when they are called, so you
shouldn't use them to implement cleanup that has to be deterministic or can
fail.

[-- Attachment #2: Type: text/html, Size: 3251 bytes --]

Re: Module support: No environment in pointer release function

[Elias Mårtenson]

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

On 27 February 2017 at 00:23, Philipp Stephani <p.stephani2@gmail.com>
wrote:

>
> Elias Mårtenson <lokedhs@gmail.com> schrieb am Fr., 10. Feb. 2017 um
> 06:21 Uhr:
>


> Right now I simply call abort() if there is an error. Excessive, I know,
>> but at least I know when there is a problem and the stack trace gives me
>> all the answers I need.
>>
>> I'm not sure I should keep the abort() in the final version though.
>>
>
> Probably not. You should either log (on stderr) and ignore the error, or
> not call gss_release_name (or any other function that can fail) in a
> finalizer.
> Just like they Java counterpart, finalizers can't fail. There's also no
> guarantee that they ever be called, or when they are called, so you
> shouldn't use them to implement cleanup that has to be deterministic or can
> fail.
>

Thank you. That seems reasonable, and I will do this.

Regards,
Elias

[-- Attachment #2: Type: text/html, Size: 2338 bytes --]