You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by GitBox <gi...@apache.org> on 2022/03/28 23:31:31 UTC
[GitHub] [jackrabbit-oak] Ewocker opened a new pull request #530: Add check for StackOverflowError
Ewocker opened a new pull request #530:
URL: https://github.com/apache/jackrabbit-oak/pull/530
This PR prevent `StackOverflowError` for recursive call between function `isIndexActive` and `isIndexActiveMerged`.
--
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: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] nit0906 commented on a change in pull request #530: Add check for StackOverflowError
Posted by GitBox <gi...@apache.org>.
nit0906 commented on a change in pull request #530:
URL: https://github.com/apache/jackrabbit-oak/pull/530#discussion_r839143916
##########
File path: oak-search/src/test/resources/logback-test.xml
##########
@@ -0,0 +1,38 @@
+<!--
Review comment:
ok
--
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: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] thomasmueller merged pull request #530: Add check for StackOverflowError
Posted by GitBox <gi...@apache.org>.
thomasmueller merged pull request #530:
URL: https://github.com/apache/jackrabbit-oak/pull/530
--
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: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] Ewocker commented on pull request #530: Add check for StackOverflowError
Posted by GitBox <gi...@apache.org>.
Ewocker commented on pull request #530:
URL: https://github.com/apache/jackrabbit-oak/pull/530#issuecomment-1082515511
> We are missing a test case for this.
added test case
--
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: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] Ewocker commented on a change in pull request #530: Add check for StackOverflowError
Posted by GitBox <gi...@apache.org>.
Ewocker commented on a change in pull request #530:
URL: https://github.com/apache/jackrabbit-oak/pull/530#discussion_r838026795
##########
File path: oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/IndexName.java
##########
@@ -233,16 +233,21 @@ public String nextCustomizedName() {
private static boolean isIndexActive(String indexPath, NodeState rootState) {
// An index is active if it has a hidden child node that starts with ":oak:mount-",
// OR if it is an active merged index
- NodeState indexNode = rootState;
- for(String e : PathUtils.elements(indexPath)) {
- indexNode = indexNode.getChildNode(e);
- }
- for(String c : indexNode.getChildNodeNames()) {
- if (c.startsWith(":oak:mount-")) {
- return true;
+ try {
+ NodeState indexNode = rootState;
+ for (String e : PathUtils.elements(indexPath)) {
+ indexNode = indexNode.getChildNode(e);
}
+ for (String c : indexNode.getChildNodeNames()) {
+ if (c.startsWith(":oak:mount-")) {
+ return true;
+ }
+ }
+ return isIndexActiveMerged(indexNode, rootState);
+ } catch (StackOverflowError e) {
+ LOG.error("Fail to check index activeness for {} with error {}", indexPath, e);
}
- return isIndexActiveMerged(indexNode, rootState);
+ return true;
Review comment:
done
--
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: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] Ewocker commented on a change in pull request #530: Add check for StackOverflowError
Posted by GitBox <gi...@apache.org>.
Ewocker commented on a change in pull request #530:
URL: https://github.com/apache/jackrabbit-oak/pull/530#discussion_r839141746
##########
File path: oak-search/src/test/resources/logback-test.xml
##########
@@ -0,0 +1,38 @@
+<!--
Review comment:
I believe I need this to use the LogCustomizer for comparing my log output
--
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: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] nit0906 commented on a change in pull request #530: Add check for StackOverflowError
Posted by GitBox <gi...@apache.org>.
nit0906 commented on a change in pull request #530:
URL: https://github.com/apache/jackrabbit-oak/pull/530#discussion_r839124847
##########
File path: oak-search/src/test/resources/logback-test.xml
##########
@@ -0,0 +1,38 @@
+<!--
Review comment:
Why do we need this separate logback config for ?
--
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: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] thomasmueller commented on a change in pull request #530: Add check for StackOverflowError
Posted by GitBox <gi...@apache.org>.
thomasmueller commented on a change in pull request #530:
URL: https://github.com/apache/jackrabbit-oak/pull/530#discussion_r837113921
##########
File path: oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/IndexName.java
##########
@@ -233,16 +233,21 @@ public String nextCustomizedName() {
private static boolean isIndexActive(String indexPath, NodeState rootState) {
// An index is active if it has a hidden child node that starts with ":oak:mount-",
// OR if it is an active merged index
- NodeState indexNode = rootState;
- for(String e : PathUtils.elements(indexPath)) {
- indexNode = indexNode.getChildNode(e);
- }
- for(String c : indexNode.getChildNodeNames()) {
- if (c.startsWith(":oak:mount-")) {
- return true;
+ try {
+ NodeState indexNode = rootState;
+ for (String e : PathUtils.elements(indexPath)) {
+ indexNode = indexNode.getChildNode(e);
}
+ for (String c : indexNode.getChildNodeNames()) {
+ if (c.startsWith(":oak:mount-")) {
+ return true;
+ }
+ }
+ return isIndexActiveMerged(indexNode, rootState);
+ } catch (StackOverflowError e) {
+ LOG.error("Fail to check index activeness for {} with error {}", indexPath, e);
}
- return isIndexActiveMerged(indexNode, rootState);
+ return true;
Review comment:
OK, so if we get a StackOverflowError, then the index is marked active... This is OK in my view. I think it would be clearer if the "return" is moved into the "catch" part.
--
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: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org