Skip to content

Commit

Permalink
Try to add a convenient way to spot recursion in callbacks. Probably …
Browse files Browse the repository at this point in the history
…should do the same thing for the ged_exec cmdnames directly.
  • Loading branch information
starseeker committed Nov 26, 2024
1 parent 76a6b19 commit d9e0032
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 9 deletions.
12 changes: 12 additions & 0 deletions include/ged/defines.h
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,18 @@ GED_EXPORT extern int ged_clbk_set(struct ged *gedp, const char *cmd, int mode,
// Method calling code can use to get the current clbk info for a specific command.
GED_EXPORT extern int ged_clbk_get(bu_clbk_t *f, void **d, struct ged *gedp, const char *cmd, int mode);

// Wrapper that utilizes internal libged capabilities to detect recursive callbacks
// Using this is optional - caller can also use results of ged_clbk_get directly -
// but if they wish to detect and report recursion this functionality is helpful.
GED_EXPORT extern int ged_clbk_exec(
struct bu_vls *log,
struct ged *gedp,
int limit,
bu_clbk_t f,
int ac,
const char **av,
void *u1,
void *u2);

// Functions for associating and retrieving application context information for
// specific dm types. Not really using this yet, but we will eventually need
Expand Down
21 changes: 13 additions & 8 deletions src/libged/exec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,16 @@ ged_exec(struct ged *gedp, int argc, const char *argv[])
const struct ged_cmd *cmd = c_it->second;


// We have a command now - check for a pre-exec callback.
// We have a command now - push it onto the stack
GED_CK_MAGIC(gedp);
Ged_Internal *gedip = gedp->i->i;
gedip->exec_stack.push(cmdname);

// Check for a pre-exec callback.
bu_clbk_t f = NULL;
void *d = NULL;
if ((ged_clbk_get(&f, &d, gedp, cmdname.c_str(), GED_CLBK_PRE) == BRLCAD_OK) && f) {
// TODO - should probably have some recursive guards here... counters in
// the internal state or some such...
cret = (*f)(argc, argv, gedp, d);
cret = ged_clbk_exec(gedp->ged_result_str, gedp, GED_CMD_RECURSION_LIMIT, f, argc, argv, gedp, d);
if (cret != BRLCAD_OK)
bu_log("error running %s pre-execution callback\n", cmdname.c_str());
}
Expand All @@ -104,26 +107,28 @@ ged_exec(struct ged *gedp, int argc, const char *argv[])
// Preliminaries complete - do the actual command execution call
cret = (*cmd->i->cmd)(gedp, argc, argv);

// If we didn't execute successfully, don't execute the post run hook
// If we didn't execute successfully, don't execute the post run hook. (If
// a specific command wants to anyway, it can do so in its own
// implementation.)
if (cret != BRLCAD_OK) {
if (tstr)
bu_log("%s time: %g\n", cmdname.c_str(), (bu_gettime() - start)/1e6);

gedip->exec_stack.pop();
return cret;
}

// Command execution complete - check for a post command callback.
if ((ged_clbk_get(&f, &d, gedp, cmdname.c_str(), GED_CLBK_POST) == BRLCAD_OK) && f) {
// TODO - should probably have some recursive guards here... counters in
// the internal state or some such...
cret = (*f)(argc, argv, gedp, d);
cret = ged_clbk_exec(gedp->ged_result_str, gedp, GED_CMD_RECURSION_LIMIT, f, argc, argv, gedp, d);
if (cret != BRLCAD_OK)
bu_log("error running %s post-execution callback\n", cmdname.c_str());
}

if (tstr)
bu_log("%s time: %g\n", cmdname.c_str(), (bu_gettime() - start)/1e6);

gedip->exec_stack.pop();
return cret;
}

Expand Down
2 changes: 1 addition & 1 deletion src/libged/facetize/subprocess/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -464,8 +464,8 @@ facetize_process(int argc, const char **argv)
return BRLCAD_OK;
}

extern "C" {
#include "../../include/plugin.h"
extern "C" {
struct ged_cmd_process_impl fp_impl = {
facetize_process
};
Expand Down
33 changes: 33 additions & 0 deletions src/libged/ged.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,39 @@ _ged_open_dbip(const char *filename, int existing_only)

/* Callback wrapper functions */

int
ged_clbk_exec(struct bu_vls *log, struct ged *gedp, int limit, bu_clbk_t f, int ac, const char **av, void *u1, void *u2)
{
if (!gedp || !f)
return BRLCAD_ERROR;
GED_CK_MAGIC(gedp);
Ged_Internal *gedip = gedp->i->i;
int rlimit = (limit > 0) ? limit : 1;

gedip->recursion_depth_cnt[f]++;

if (gedip->recursion_depth_cnt[f] > rlimit) {
if (log) {
// Print out ged_exec call stack that got us here. If the
// recursion is all in callback functions this won't help, but at
// the very least we'll know which ged command to start with.
bu_vls_printf(log, "Callback recursion limit %d exceeded. ged_exec call stack:\n", rlimit);
std::stack<std::string> lexec_stack = gedip->exec_stack;
while (!lexec_stack.empty()) {
bu_vls_printf(log, "%s\n", lexec_stack.top().c_str());
lexec_stack.pop();
}
}
return BRLCAD_ERROR;
}

int ret = (*f)(ac, av, u1, u2);

gedip->recursion_depth_cnt[f]++;

return ret;
}

void
ged_refresh_cb(struct ged *gedp)
{
Expand Down
9 changes: 9 additions & 0 deletions src/libged/ged_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@

#ifdef __cplusplus

#include <map>
#include <stack>
#include <string>

class Ged_Internal {
public:
struct ged *gedp;
Expand All @@ -52,6 +56,10 @@ class Ged_Internal {
std::map<ged_func_ptr, std::pair<bu_clbk_t, void *>> cmd_postrun_clbk;
std::map<ged_func_ptr, std::pair<bu_clbk_t, void *>> cmd_linger_clbk;

std::map<bu_clbk_t, int> recursion_depth_cnt;

std::stack<std::string> exec_stack;

std::map<std::string, void *> dm_map;
};

Expand Down Expand Up @@ -109,6 +117,7 @@ __BEGIN_DECLS
/* Default libged column width assumption */
#define GED_TERMINAL_WIDTH 80

#define GED_CMD_RECURSION_LIMIT 100

/* Callback management related structures */
#define GED_REFRESH_FUNC_NULL ((ged_refresh_func_t)0)
Expand Down

0 comments on commit d9e0032

Please sign in to comment.