From 7d59da0c160c7b4b288fbbfb5c2c1d17049372c5 Mon Sep 17 00:00:00 2001 From: Sam Hartman Date: Wed, 11 Mar 2015 10:15:59 -0400 Subject: [PATCH] Connection clean up We need to clean up the connection file descriptor and the gss context. * tidc_fwd_request is the wrong place to free the TID_REQ because it does not allocate it * tid_req_dup needs to use talloc; duplicated requests are freed by the original request * tidc_send_message frees its request * The request destructor closes the connection. * Keep track of whether a request is duplicated; only free the connection and gss context in the original request. --- include/tid_internal.h | 1 + tid/tid_req.c | 16 ++++++++++---- tid/tidc.c | 60 ++++++++++++++++++++++++++++---------------------- tr/tr_main.c | 2 ++ 4 files changed, 49 insertions(+), 30 deletions(-) diff --git a/include/tid_internal.h b/include/tid_internal.h index 525e671..8502185 100644 --- a/include/tid_internal.h +++ b/include/tid_internal.h @@ -64,6 +64,7 @@ struct tid_req { struct tid_req *next_req; int resp_sent; int conn; + int free_conn; /* free conn and gss ctx*/ gss_ctx_id_t gssctx; int resp_rcvd; TR_NAME *rp_realm; diff --git a/tid/tid_req.c b/tid/tid_req.c index a1a7a5a..1da8338 100644 --- a/tid/tid_req.c +++ b/tid/tid_req.c @@ -33,6 +33,7 @@ */ #include +#include #include #include #include @@ -44,11 +45,16 @@ static int destroy_tid_req(TID_REQ *req) { - OM_uint32 minor; if (req->json_references) json_decref(req->json_references); - if (req->gssctx) - gss_delete_sec_context( &minor, &req->gssctx, NULL); + if (req->free_conn) { + if (req->conn) + close(req->conn); + if (req->gssctx) { + OM_uint32 minor; + gss_delete_sec_context( &minor, &req->gssctx, NULL); + } + } return 0; } @@ -61,6 +67,7 @@ TID_REQ *tid_req_new() req->json_references = json_array(); assert(req->json_references); req->conn = -1; + req->free_conn = 1; return req; } @@ -178,7 +185,7 @@ TID_REQ *tid_dup_req (TID_REQ *orig_req) { TID_REQ *new_req = NULL; - if (NULL == (new_req = malloc(sizeof(TID_REQ)))) { + if (NULL == (new_req = talloc_zero(orig_req, TID_REQ))) { tr_crit("tid_dup_req: Can't allocated duplicate request."); return NULL; } @@ -186,6 +193,7 @@ TID_REQ *tid_dup_req (TID_REQ *orig_req) /* Memcpy for flat fields, not valid until names are duped. */ memcpy(new_req, orig_req, sizeof(TID_REQ)); json_incref(new_req->json_references); + new_req->free_conn = 0; if ((NULL == (new_req->rp_realm = tr_dup_name(orig_req->rp_realm))) || (NULL == (new_req->realm = tr_dup_name(orig_req->realm))) || diff --git a/tid/tidc.c b/tid/tidc.c index f0f0cad..62968fb 100644 --- a/tid/tidc.c +++ b/tid/tidc.c @@ -40,6 +40,8 @@ #include #include #include +#include + int tmp_len = 32; @@ -88,9 +90,9 @@ int tidc_send_request (TIDC_INSTANCE *tidc, char *comm, TIDC_RESP_FUNC *resp_handler, void *cookie) - { TID_REQ *tid_req = NULL; + int rc; /* Create and populate a TID req structure */ if (!(tid_req = tid_req_new())) @@ -102,20 +104,25 @@ int tidc_send_request (TIDC_INSTANCE *tidc, if ((NULL == (tid_req->rp_realm = tr_new_name(rp_realm))) || (NULL == (tid_req->realm = tr_new_name(realm))) || (NULL == (tid_req->comm = tr_new_name(comm)))) { - fprintf (stderr, "tidc_send_request: Error duplicating names.\n"); - return -1; + tr_err ( "tidc_send_request: Error duplicating names.\n"); + goto error; } tid_req->tidc_dh = tidc->client_dh; - return (tidc_fwd_request(tidc, tid_req, resp_handler, cookie)); + rc = tidc_fwd_request(tidc, tid_req, resp_handler, cookie); + goto cleanup; + error: + rc = -1; + cleanup: + tid_req_free(tid_req); + return rc; } int tidc_fwd_request (TIDC_INSTANCE *tidc, TID_REQ *tid_req, TIDC_RESP_FUNC *resp_handler, void *cookie) - { char *req_buf = NULL; char *resp_buf = NULL; @@ -123,10 +130,11 @@ int tidc_fwd_request (TIDC_INSTANCE *tidc, TR_MSG *msg = NULL; TR_MSG *resp_msg = NULL; int err; + int rc = 0; /* Create and populate a TID msg structure */ - if (!(msg = malloc(sizeof(TR_MSG)))) - return -1; + if (!(msg = talloc_zero(tid_req, TR_MSG))) + goto error; msg->msg_type = TID_REQUEST; tr_msg_set_req(msg, tid_req); @@ -138,18 +146,18 @@ int tidc_fwd_request (TIDC_INSTANCE *tidc, /* Encode the request into a json string */ if (!(req_buf = tr_msg_encode(msg))) { - fprintf(stderr, "tidc_fwd_request: Error encoding TID request.\n"); - return -1; + tr_err("tidc_fwd_request: Error encoding TID request.\n"); + goto error; } - fprintf (stderr, "tidc_fwd_request: Sending TID request:\n"); - fprintf (stderr, "%s\n", req_buf); + tr_debug( "tidc_fwd_request: Sending TID request:\n"); + tr_debug( "%s\n", req_buf); /* Send the request over the connection */ if (err = gsscon_write_encrypted_token (tid_req->conn, tid_req->gssctx, req_buf, strlen(req_buf))) { - fprintf(stderr, "tidc_fwd_request: Error sending request over connection.\n"); - return -1; + tr_err( "tidc_fwd_request: Error sending request over connection.\n"); + goto error; } /* TBD -- queue request on instance, read resps in separate thread */ @@ -159,33 +167,33 @@ int tidc_fwd_request (TIDC_INSTANCE *tidc, if (err = gsscon_read_encrypted_token(tid_req->conn, tid_req->gssctx, &resp_buf, &resp_buflen)) { if (resp_buf) free(resp_buf); - return -1; + goto error; } - fprintf(stdout, "tidc_fwd_request: Response Received (%u bytes).\n", (unsigned) resp_buflen); - fprintf(stdout, "%s\n", resp_buf); + tr_debug( "tidc_fwd_request: Response Received (%u bytes).\n", (unsigned) resp_buflen); + tr_debug( "%s\n", resp_buf); if (NULL == (resp_msg = tr_msg_decode(resp_buf, resp_buflen))) { - fprintf(stderr, "tidc_fwd_request: Error decoding response.\n"); - return -1; + tr_err( "tidc_fwd_request: Error decoding response.\n"); + goto error; } /* TBD -- Check if this is actually a valid response */ if (TID_RESPONSE != tr_msg_get_msg_type(resp_msg)) { - fprintf(stderr, "tidc_fwd_request: Error, no response in the response!\n"); - return -1; + tr_err( "tidc_fwd_request: Error, no response in the response!\n"); + goto error; } if (resp_handler) /* Call the caller's response function */ (*resp_handler)(tidc, tid_req, tr_msg_get_resp(resp_msg), cookie); - else - fprintf(stderr, "tidc_fwd_request: NULL response function.\n"); + goto cleanup; + error: + rc = -1; + cleanup: if (msg) - free(msg); - if (tid_req) - tid_req_free(tid_req); + talloc_free(msg); if (req_buf) free(req_buf); if (resp_buf) @@ -193,7 +201,7 @@ int tidc_fwd_request (TIDC_INSTANCE *tidc, /* TBD -- free the decoded response */ - return 0; + return rc; } diff --git a/tr/tr_main.c b/tr/tr_main.c index e77d8aa..ae662fa 100644 --- a/tr/tr_main.c +++ b/tr/tr_main.c @@ -216,9 +216,11 @@ static int tr_tids_req_handler (TIDS_INSTANCE *tids, if (0 > (rc = tidc_fwd_request(tidc, fwd_req, &tr_tidc_resp_handler, (void *)&resp_cookie))) { tr_notice("Error from tidc_fwd_request, rc = %d.", rc); tids_send_err_response(tids, orig_req, "Can't forward request to next hop TIDS"); + tid_req_free(orig_req); return -1; } + tid_req_free(orig_req); return 0; } -- 2.1.4