You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2021/11/22 22:49:28 UTC
[GitHub] [couchdb] lostnet opened a new pull request #3842: feat(couchjs): add support for SpiderMonkey 91esr
lostnet opened a new pull request #3842:
URL: https://github.com/apache/couchdb/pull/3842
## Overview
This PR introduces spidermonkey 91esr. As with 78 and 86 there are no relevant API changes but it became necessary in practice as well as in theory to destroy the context and shutdown JS before any proper exit to prevent assertions in destructors. (For more information, the issue tracking the esr91 migration guide is [here](https://github.com/mozilla-spidermonkey/spidermonkey-embedding-examples/issues/41.)
## Testing recommendations
Since many distributions don't ship with moz-js 91 yet, I've included a github workflow to test couchdb in ubuntu with the latest spidermonkey, foundationdb 6.x, and icu. It is set to be manually run on a branch from the actions menu.
## Related Issues or Pull Requests
<!-- If your changes affects multiple components in different
repositories please put links to those issues or pull requests here. -->
## Checklist
- [X] Code is written and works correctly
- [X] Changes are covered by tests
- [X] Any new configurable parameters are documented in `rel/overlay/etc/default.ini` [^1]
- [X] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation [^2]
[^1]: no configurable parameters for runtime
[^2]: spidermonkey versions are only referenced in whatsnew for release branches.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] lostnet commented on a change in pull request #3842: feat(couchjs): add support for SpiderMonkey 91esr
Posted by GitBox <gi...@apache.org>.
lostnet commented on a change in pull request #3842:
URL: https://github.com/apache/couchdb/pull/3842#discussion_r761202765
##########
File path: src/couch/priv/couch_js/86/util.cpp
##########
@@ -330,7 +330,7 @@ void
couch_oom(JSContext* cx, void* data)
{
fprintf(stderr, "out of memory\n");
- exit(1);
+ _Exit(1);
Review comment:
It is a little of both something that changed and a bug that existed.. Technically it was never correct to do a proper exit() without cleaning up the js context which we can't do in the middle of being called from the js context (and having a shortage of memory its risky to assume we can get a chance to exit later), but it was only in recent versions that mozilla added destructors that notice they are being called with a still active context.
Since it was a bug I saw no problem with keeping it combined and fixing 86 and 78 in the process, but none of the earlier versions should ever get patches again so it probably doesn't need to be backported to the earlier wrappers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] lostnet commented on pull request #3842: feat(couchjs): add support for SpiderMonkey 91esr
Posted by GitBox <gi...@apache.org>.
lostnet commented on pull request #3842:
URL: https://github.com/apache/couchdb/pull/3842#issuecomment-976674086
I don't see how the erlfdb test failure relates to the PR. If this is from [fdb errors](https://apple.github.io/foundationdb/api-error-codes.html) it is `key_outside_legal_range` which apparently could make sense with a random key since some keys are special but it is highly unlikely to happen with multiple runs if they are getting different random seeds?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on pull request #3842: feat(couchjs): add support for SpiderMonkey 91esr
Posted by GitBox <gi...@apache.org>.
nickva commented on pull request #3842:
URL: https://github.com/apache/couchdb/pull/3842#issuecomment-976795493
@lostnet Thanks for your contribution!
I believe you just hit a spurious bug in erlfdb. That when we generate random keys and value to add but forgot guard against writing to \xFF range.
I had created an issue to fix it https://github.com/apache/couchdb-erlfdb/issues/46
Re-triggered your PR and now it passed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] kocolosk merged pull request #3842: feat(couchjs): add support for SpiderMonkey 91esr
Posted by GitBox <gi...@apache.org>.
kocolosk merged pull request #3842:
URL: https://github.com/apache/couchdb/pull/3842
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] lostnet edited a comment on pull request #3842: feat(couchjs): add support for SpiderMonkey 91esr
Posted by GitBox <gi...@apache.org>.
lostnet edited a comment on pull request #3842:
URL: https://github.com/apache/couchdb/pull/3842#issuecomment-976674086
I don't see how the erlfdb test failure relates to the PR. If this is from [fdb errors](https://apple.github.io/foundationdb/api-error-codes.html) it is `key_outside_legal_range` which apparently could make sense with a random key since some keys are special ~~but it is highly unlikely to happen with multiple runs if they are getting different random seeds?~~ (I think I misread the results and the error seems to only have occur in one actual 23.3.1 run.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] kocolosk commented on a change in pull request #3842: feat(couchjs): add support for SpiderMonkey 91esr
Posted by GitBox <gi...@apache.org>.
kocolosk commented on a change in pull request #3842:
URL: https://github.com/apache/couchdb/pull/3842#discussion_r761155928
##########
File path: src/couch/priv/couch_js/86/util.cpp
##########
@@ -330,7 +330,7 @@ void
couch_oom(JSContext* cx, void* data)
{
fprintf(stderr, "out of memory\n");
- exit(1);
+ _Exit(1);
Review comment:
Is this change something that ought to be backported to our wrappers for older versions of SpiderMonkey, or is it motivated by something specific in ESR91?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org