Leak fixes by jeffreylovitz · Pull Request #935 · RedisGraph/RedisGraph · GitHub
Skip to content

Leak fixes #935

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
swilly22 merged 4 commits into from
Feb 20, 2020
Merged

Leak fixes #935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f6a179c
Close Redis key handles
jeffreylovitz Feb 19, 2020
b93f3de
Fix memory leak in checking whether procedures are read-only
jeffreylovitz Feb 19, 2020
0052f53
Improve explanatory comment
jeffreylovitz Feb 20, 2020
078de18
Merge branch 'master' into leak-fixes
swilly22 Feb 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/ast/ast.c
View file
Open in desktop
4 changes: 3 additions & 1 deletion src/commands/cmd_delete.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ int MGraph_Delete(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
// Remove graph from keyspace.
RedisModuleKey *key = RedisModule_OpenKey(ctx, graph_name, REDISMODULE_WRITE);
RedisModule_DeleteKey(key); // Decreases graph ref count.
GraphContext_Release(gc); // Decrease graph ref count.
RedisModule_CloseKey(key); // Free key handle.
GraphContext_Release(gc); // Decrease graph ref count.

double t = QueryCtx_GetExecutionTime();
asprintf(&strElapsed, "Graph removed, internal execution time: %.6f milliseconds", t);
Expand All @@ -46,3 +47,4 @@ int MGraph_Delete(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
RedisModule_ReplicateVerbatim(ctx);
return REDISMODULE_OK;
}

13 changes: 13 additions & 0 deletions src/procedures/procedure.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,18 @@ bool Procedure_ContainsOutput(const ProcedureCtx *proc, const char *output) {
return false;
}

bool Proc_ReadOnly(const char *proc_name) {
assert(__procedures);
ProcGenerator gen = raxFind(__procedures, (unsigned char *)proc_name, strlen(proc_name));
if(gen == raxNotFound) return false; // Invalid procedure specified, handled elsewhere.
/* TODO It would be preferable to be able to determine whether a procedure is read-only
* without creating its entire context; this is wasteful. */
Comment on lines +125 to +126

@swilly22 swilly22 Feb 20, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree

ProcedureCtx *ctx = gen();
bool read_only = ctx->readOnly;
Proc_Free(ctx);
return read_only;
}

void Proc_Free(ProcedureCtx *proc) {
if(!proc) return;
proc->Free(proc);
Expand All @@ -129,3 +141,4 @@ void Proc_Free(ProcedureCtx *proc) {

rm_free(proc);
}

6 changes: 5 additions & 1 deletion src/procedures/procedure.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,15 @@ uint Procedure_Argc(const ProcedureCtx *proc);
/* Return number of outputs yield by procedure. */
uint Procedure_OutputCount(const ProcedureCtx *proc);

/* Retrieves procedure ith output */
/* Return the name of the procedure's output at position output_idx. */
const char *Procedure_GetOutput(const ProcedureCtx *proc, uint output_idx);

/* Returns true if given output can be yield by procedure */
bool Procedure_ContainsOutput(const ProcedureCtx *proc, const char *output);

/* Returns true if procedure is read-only. */
bool Proc_ReadOnly(const char *proc_name);

@swilly22 swilly22 Feb 20, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition.


// Free procedure context.
void Proc_Free(ProcedureCtx *proc);

6 changes: 4 additions & 2 deletions src/query_ctx.c
View file
Open in desktop