{"thread":{"id":"10753","subject":"[PATCH 06/11] git-fetch: Release objects used by a prior transport","startedAt":"2007-11-09T11:06:26Z","lastAt":"2007-11-11T05:29:49Z","messageCount":3,"participants":["Shawn O. Pearce","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":11},"messages":[{"id":"59022","messageId":"20071109110626.GF19368@spearce.org","threadId":"10753","inReplyTo":null,"subject":"[PATCH 06/11] git-fetch: Release objects used by a prior transport","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-11-09T11:06:26Z","receivedAt":"2007-11-09T11:06:26Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Some transports allocate objects in the internal object hashtable\nduring the fetch process (e.g. the HTTP commit walker and also the\nnative protocol). These shouldn't be visible to another transport\ncall running in the same fetch process when we fetch the tags during\nautomated tag following. By deallocating the object table (if it\nhas anything in it) we ensure the second transport execution will\nbe from a clean slate.\n\nSigned-off-by: Shawn O. Pearce \n---\n builtin-fetch.c | 5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex 847db73..18f123e 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -337,7 +337,10 @@ static void store_updated_refs(const char *url, struct ref *ref_map)\n \n static int fetch_refs(struct transport *transport, struct ref *ref_map)\n {\n-\tint ret = transport_fetch_refs(transport, ref_map);\n+\tint ret;\n+\n+\tfree_all_objects();\n+\tret = transport_fetch_refs(transport, ref_map);\n \tif (!ret)\n \t\tstore_updated_refs(transport->url, ref_map);\n \ttransport_unlock_pack(transport);\n-- \n1.5.3.5.1622.g41d10\n"},{"id":"59131","messageId":"7vve8bqcke.fsf@gitster.siamese.dyndns.org","threadId":"10753","inReplyTo":"20071109110626.GF19368@spearce.org","subject":"Re: [PATCH 06/11] git-fetch: Release objects used by a prior transport","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-09T22:27:45Z","receivedAt":"2007-11-09T22:27:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Some transports allocate objects in the internal object hashtable\n> during the fetch process (e.g. the HTTP commit walker and also the\n> native protocol). These shouldn't be visible to another transport\n> call running in the same fetch process when we fetch the tags during\n> automated tag following. By deallocating the object table (if it\n> has anything in it) we ensure the second transport execution will\n> be from a clean slate.\n>\n> Signed-off-by: Shawn O. Pearce \n> ---\n> builtin-fetch.c | 5 ++++-\n> 1 files changed, 4 insertions(+), 1 deletions(-)\n>\n> diff --git a/builtin-fetch.c b/builtin-fetch.c\n> index 847db73..18f123e 100644\n> --- a/builtin-fetch.c\n> +++ b/builtin-fetch.c\n> @@ -337,7 +337,10 @@ static void store_updated_refs(const char *url, struct ref *ref_map)\n> \n> static int fetch_refs(struct transport *transport, struct ref *ref_map)\n> {\n> -\tint ret = transport_fetch_refs(transport, ref_map);\n> +\tint ret;\n> +\n> +\tfree_all_objects();\n> +\tret = transport_fetch_refs(transport, ref_map);\n> \tif (!ret)\n> \t\tstore_updated_refs(transport->url, ref_map);\n> \ttransport_unlock_pack(transport);\n\nThis sounds a very heavy-handed approach.\n\nIs it the callers responsibility to know what function does call\nfree_all_objects() and makes sure there is no pointer to objects\nobtained before the call that is used after the call returns?\n"},{"id":"59266","messageId":"20071111052949.GU14735@spearce.org","threadId":"10753","inReplyTo":"7vve8bqcke.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 06/11] git-fetch: Release objects used by a prior transport","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-11-11T05:29:49Z","receivedAt":"2007-11-11T05:29:49Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> \"Shawn O. Pearce\" writes:\n> > diff --git a/builtin-fetch.c b/builtin-fetch.c\n> > index 847db73..18f123e 100644\n> > --- a/builtin-fetch.c\n> > +++ b/builtin-fetch.c\n> > @@ -337,7 +337,10 @@ static void store_updated_refs(const char *url, struct ref *ref_map)\n> > \n> > static int fetch_refs(struct transport *transport, struct ref *ref_map)\n> > {\n> > -\tint ret = transport_fetch_refs(transport, ref_map);\n> > +\tint ret;\n> > +\n> > +\tfree_all_objects();\n> > +\tret = transport_fetch_refs(transport, ref_map);\n> > \tif (!ret)\n> > \t\tstore_updated_refs(transport->url, ref_map);\n> > \ttransport_unlock_pack(transport);\n> \n> This sounds a very heavy-handed approach.\n> \n> Is it the callers responsibility to know what function does call\n> free_all_objects() and makes sure there is no pointer to objects\n> obtained before the call that is used after the call returns?\n\nYea, I guess it is. That's part of the reason why this usage was\nput here in a static function of builtin-fetch. Its high enough\nup in the call stack that nobody above it cares. Where we would\nrun into trouble would be if a transport decided to hang onto any\npointers in its data structure between calls to the transport.\n\nLets just say we don't free the objects; the flags are still all\nmessed up from any prior user. Its tricky to reuse the objects\nbecause we don't know what state an object is left in by someone\nthat ran before us (e.g. an internal call to fetch-pack!). Its\nalso tricky when they free the parent list but leave parsed=1;\nwe can't rebuild the parent pointers!\n\n-- \nShawn.\n"}]}