You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by "Mike Hommey (JIRA)" <ji...@apache.org> on 2009/10/24 15:57:59 UTC

[jira] Created: (COUCHDB-542) Fix for COUCHDB-288 makes JS_MaybeGC not called

Fix for COUCHDB-288 makes JS_MaybeGC not called
-----------------------------------------------

                 Key: COUCHDB-542
                 URL: https://issues.apache.org/jira/browse/COUCHDB-542
             Project: CouchDB
          Issue Type: Bug
          Components: JavaScript View Server
            Reporter: Mike Hommey


The fix for COUCHDB-288 only basically replaces JS_SetBranchCallback with JS_SetOperationCallback, which is not enough for the callback to be triggered. The problem is that basically, the operation callback API has now nothing to do with the previous branch callback API, and is not called at each branch call at the JS level. Actually, it is not called at all, except if JS_TriggerOperationCallback is used. Typically, this needs to be done either from a signal handler or a watchdog thread, in which case the test inside the callback is pretty pointless.

See http://groups.google.com/group/mozilla.dev.tech.js-engine/browse_thread/thread/a4d1fe147761aacb/e61d2592faf4ef72?lnk=gst&q=js_setoperationcallback#e61d2592faf4ef72 for reference.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (COUCHDB-542) Fix for COUCHDB-288 makes JS_MaybeGC not called

Posted by "Paul Joseph Davis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-542?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12769836#action_12769836 ] 

Paul Joseph Davis commented on COUCHDB-542:
-------------------------------------------

The view servers are run in their own OS level process and communicated with over stdio. Thus when we kill the process its up to the kernel to make sure all is well. I'm toying around with refactoring couchjs so that I can work on the command line tests. I'll look at making those changes as well and see how things go.

> Fix for COUCHDB-288 makes JS_MaybeGC not called
> -----------------------------------------------
>
>                 Key: COUCHDB-542
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-542
>             Project: CouchDB
>          Issue Type: Bug
>          Components: JavaScript View Server
>            Reporter: Mike Hommey
>
> The fix for COUCHDB-288 only basically replaces JS_SetBranchCallback with JS_SetOperationCallback, which is not enough for the callback to be triggered. The problem is that basically, the operation callback API has now nothing to do with the previous branch callback API, and is not called at each branch call at the JS level. Actually, it is not called at all, except if JS_TriggerOperationCallback is used. Typically, this needs to be done either from a signal handler or a watchdog thread, in which case the test inside the callback is pretty pointless.
> See http://groups.google.com/group/mozilla.dev.tech.js-engine/browse_thread/thread/a4d1fe147761aacb/e61d2592faf4ef72?lnk=gst&q=js_setoperationcallback#e61d2592faf4ef72 for reference.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (COUCHDB-542) Fix for COUCHDB-288 makes JS_MaybeGC not called

Posted by "Mike Hommey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-542?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12769768#action_12769768 ] 

Mike Hommey commented on COUCHDB-542:
-------------------------------------

This sounds reasonable, though I'd have a comment to add to what you describe as the current behaviour for runaway scripts. Disclaimer: I don't know the couchdb code, nor spidermonkey deep internals. I'm afraid there could be some subtle memory leaks if you forcibly terminate the javascript engine without going through the operationcallback, which could be used to gracefully terminate the engine. The latter would be better, I think, except if what you mean by forcibly terminating by the Erlang VM is that a separate process, handling javascript, is killed, in which case, there's no cleanup to do.

> Fix for COUCHDB-288 makes JS_MaybeGC not called
> -----------------------------------------------
>
>                 Key: COUCHDB-542
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-542
>             Project: CouchDB
>          Issue Type: Bug
>          Components: JavaScript View Server
>            Reporter: Mike Hommey
>
> The fix for COUCHDB-288 only basically replaces JS_SetBranchCallback with JS_SetOperationCallback, which is not enough for the callback to be triggered. The problem is that basically, the operation callback API has now nothing to do with the previous branch callback API, and is not called at each branch call at the JS level. Actually, it is not called at all, except if JS_TriggerOperationCallback is used. Typically, this needs to be done either from a signal handler or a watchdog thread, in which case the test inside the callback is pretty pointless.
> See http://groups.google.com/group/mozilla.dev.tech.js-engine/browse_thread/thread/a4d1fe147761aacb/e61d2592faf4ef72?lnk=gst&q=js_setoperationcallback#e61d2592faf4ef72 for reference.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (COUCHDB-542) Fix for COUCHDB-288 makes JS_MaybeGC not called

Posted by "Paul Joseph Davis (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/COUCHDB-542?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Paul Joseph Davis resolved COUCHDB-542.
---------------------------------------

    Resolution: Not A Problem

Using the callbacks for GC'ing isn't necessary as we ensure that JS_MaybeGC is called frequently due to the Erlang timeouts on commands.

> Fix for COUCHDB-288 makes JS_MaybeGC not called
> -----------------------------------------------
>
>                 Key: COUCHDB-542
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-542
>             Project: CouchDB
>          Issue Type: Bug
>          Components: JavaScript View Server
>            Reporter: Mike Hommey
>
> The fix for COUCHDB-288 only basically replaces JS_SetBranchCallback with JS_SetOperationCallback, which is not enough for the callback to be triggered. The problem is that basically, the operation callback API has now nothing to do with the previous branch callback API, and is not called at each branch call at the JS level. Actually, it is not called at all, except if JS_TriggerOperationCallback is used. Typically, this needs to be done either from a signal handler or a watchdog thread, in which case the test inside the callback is pretty pointless.
> See http://groups.google.com/group/mozilla.dev.tech.js-engine/browse_thread/thread/a4d1fe147761aacb/e61d2592faf4ef72?lnk=gst&q=js_setoperationcallback#e61d2592faf4ef72 for reference.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (COUCHDB-542) Fix for COUCHDB-288 makes JS_MaybeGC not called

Posted by "Paul Joseph Davis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-542?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12769726#action_12769726 ] 

Paul Joseph Davis commented on COUCHDB-542:
-------------------------------------------

Mike,

Good catch on that. When I wrote the patches that ended up with what we have now I was more worried about getting the thing to compile properly for all the different versions of Spidermonkey than anything else. I didn't even dig deep enough to see that I was supposed to be setting up a signal handler to call that periodically.

The more than I read about this the more I think the proper fix is to just ixnay the whole callback situation and forget that it ever existed. For reference, my knowledge of that callback is that it's used to prevent infinite loop scripts as well as trigger garbage collection periodically.

CouchDB solves the runaway script issue by requiring that all communication to external servers complete in a defined set of time (default of 5 seconds). When a script exceeds this threshold its forcibly terminated by the Erlang VM and eventually replaced when needed.

As to garbage collection, the current protocol runs the "reset" command each time an OS process is retrieved from the pool. At this point JS_GC() is called forcing collection. JS_MaybeGC is also called for every line of input which will save us with long checkouts during things like view building and _changes filtering.

So, yeah, I think I just convinced myself that we should probably just delete the whole thing. Unless of course I'm missing a case where having JS_MaybeGC calls are required and not being called. What do you think?

> Fix for COUCHDB-288 makes JS_MaybeGC not called
> -----------------------------------------------
>
>                 Key: COUCHDB-542
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-542
>             Project: CouchDB
>          Issue Type: Bug
>          Components: JavaScript View Server
>            Reporter: Mike Hommey
>
> The fix for COUCHDB-288 only basically replaces JS_SetBranchCallback with JS_SetOperationCallback, which is not enough for the callback to be triggered. The problem is that basically, the operation callback API has now nothing to do with the previous branch callback API, and is not called at each branch call at the JS level. Actually, it is not called at all, except if JS_TriggerOperationCallback is used. Typically, this needs to be done either from a signal handler or a watchdog thread, in which case the test inside the callback is pretty pointless.
> See http://groups.google.com/group/mozilla.dev.tech.js-engine/browse_thread/thread/a4d1fe147761aacb/e61d2592faf4ef72?lnk=gst&q=js_setoperationcallback#e61d2592faf4ef72 for reference.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.