Hey all. As in general it's a bad idea to allow an event handler to generate an event, and as comments in the code seem to suggest this isn't the intention either, I was wondering about recursion in getGraphicsEvent(). In main/gevents.c, both doMouseEvent() and doKeybd() have the following line;
dd->gettingEvent = FALSE; /* avoid recursive calls */ And they reset it to TRUE before returning. The effective result of this is that the event handlers on the R side are allowed to call getGraphicsEvent(), and recurse until they eventually would run out of stack space. Though R does catch and handle that cleanly with the error; Error: evaluation nested too deeply: infinite recursion / options(expressions=)? A quick scan of the SVN logs suggests this code has been untouched since its introduction in 2004, so I'm left to wonder if this is intended behavior. It stands out as a bit of a sore thumb due to the generic check for recursion in do_getGraphicsEvent() in the same file, which will error out with error(_("recursive use of 'getGraphicsEvent' not supported")) if dd->gettingEvent is already set to TRUE. Which would suggest recursively calling it is very much not intended to be possible. To me, setting gettingEvent to FALSE seems like an easy mistake to make if you temporarily interpret gettingEvent to mean that event(s) are allowed to still come in. But the actual interpretation in do_getGraphicsEvents() is the opposite, as it's interpreted as an indicator of whether or not an event is currently being processed. I've removed the gettingEvent altering lines from both doMouseEvent() and doKeybd() to no ill effect, and doing so disabled the ability to call getGraphicsEvent() from inside one of the assigned handlers as expected. But after 12 (!) years, it's conceivable that people have come to depend on this behavior in existing scripts. Is this something that should be left alone to minimize disruption? Or should this be fixed (if it is indeed unintended) for the sake of protecting people from infinite recursions? I've included a small patch as attachment that removes the offending lines. - Richard Bodewits
Index: src/main/gevents.c =================================================================== --- src/main/gevents.c (revision 71293) +++ src/main/gevents.c (working copy) @@ -211,8 +211,6 @@ int i; SEXP handler, bvec, sx, sy, temp, result; - dd->gettingEvent = FALSE; /* avoid recursive calls */ - PROTECT(handler = findVar(install(mouseHandlers[event]), dd->eventEnv)); if (TYPEOF(handler) == PROMSXP) { handler = eval(handler, dd->eventEnv); @@ -242,7 +240,7 @@ R_FlushConsole(); } UNPROTECT(1); /* handler */ - dd->gettingEvent = TRUE; + return; } @@ -257,8 +255,6 @@ { SEXP handler, skey, temp, result; - dd->gettingEvent = FALSE; /* avoid recursive calls */ - PROTECT(handler = findVar(install(keybdHandler), dd->eventEnv)); if (TYPEOF(handler) == PROMSXP) { handler = eval(handler, dd->eventEnv); @@ -277,6 +273,6 @@ R_FlushConsole(); } UNPROTECT(1); /* handler */ - dd->gettingEvent = TRUE; + return; }
______________________________________________ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel