Created an attachment (id=616) [details]
patch

This patch should fix the bug... It wasn't the authz callout failure that
triggered the assertion, rather bad state at cleanup time after g-u-c closed
the connection.  Let me know if this helps.

Applied the patch but that didn't fix it. BUT, looking closer at things in the
debugger, I noticed that the callback function that I'm getting is
globus_gfs_acl_cas_cb(): is that just bad naming or is it assumed that some
CAS-specific stuff is to happen here with e.g. the callback_args, and that just
any callout (i.e., mine) simply won't cut it?

The callback is called with arguments (callback_arg, handle, result) where

            result = globus_error_put(globus_error_construct_error(
                    GLOBUS_GSI_AUTHZ_MODULE,
                        NULL,
                        1234,
                        __FILE__,
                        "foo_authz_callout",
                        __LINE__,
                        "FOO module denied access to %s object %s",
                        action,
                        object));

Then I do:

    callback(callback_arg, handle, result);
    return result;

I.e., the result ends up at two places. This seems a bit odd to me and a
potential source of the problem (who is in charge of the error object cleanup?).
Also, if the authorization logic worked fine and we denied access, shouldn't we
report one GLOBUS_SUCCESS (the authz logic worked fine) and one error (the authz
decision)?

I have tried the following possible combinations (result below indicates a
return value != GLOBUS_SUCCESS)

#1: callback(...,result);
    return result;

#2: callback(...,result);
    return GLOBUS_SUCCESS;

#3: callback(...,GLOBUS_SUCCESS);
    return result;

#4: callback(...,GLOBUS_SUCCESS);
    return GLOBUS_SUCCESS;

#5: /* callback(...,GLOBUS_SUCCESS); */
    return GLOBUS_SUCCESS;

#6: callback(...,result);
    return globus_error_put(globus_object_copy(globus_error_peek(result)));

Only #4 does not result in a SEGV, assertion error, or deadlock, but that one is
not so interesting for obvious reasons... ;-)

I'm going to have to test this some more, I don't see how else it would fail 
like that.  The cas func name is just the namespace we have for our gsi-authz 
based acl module, there is nothing cas specific (we just refered to the authz 
callouts as cas as a matter of habit at that point)... as far as return val and 
callback result value, the rule is that if you return failure, you must not 
call the callback, and if you return success, you must call it.

In the meantime, can you set the env GLOBUS_GRIDFTP_SERVER_DEBUG=ALL,<filename> 
and attach <filename> when the assertion occurs.

I was able to reproduce this, and the patch correctly fixes it for me.  Can you 
verify you applied the patch successfully?  If you're still asserting with the 
patch, something else must be going on.  Send me your callout and I'll try with 
that.

Mike

I doublechecked, and yes, my code works fine with your patch. As indicated
previously, I tried a bunch of different ways to properly propagate a return
value!=GLOBUS_SUCCESS, the only one I didn't try was the correct one...
 
This bug can be probably be closed, but I would suggest that a simple "if
(result) return;" statement is added at the top of the receiving callback
function, to make the system less sensitive to bad developers. :-)

Unfortunately its hard to support errors involving callbacks coming when they 
shouldn't or not coming when they should.  An failure result to the callback is 
still acceptable (and indeed, possibly the most 'correct' from an async 
standpoint), as long as the callout returned success in the first place to 
indicate a callback would come.

Thanks for testing.  The patch here will be included with the next bugfix 
release.

Mike