You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/02/04 17:53:27 UTC
[GitHub] [solr] cpoerschke opened a new pull request #597: SOLR-15983: synchronise LogUpdateProcessor's toLog member use
cpoerschke opened a new pull request #597:
URL: https://github.com/apache/solr/pull/597
https://issues.apache.org/jira/browse/SOLR-15983
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #597: SOLR-15983: synchronise LogUpdateProcessor's toLog member use
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #597:
URL: https://github.com/apache/solr/pull/597#discussion_r799742111
##########
File path: solr/core/src/java/org/apache/solr/update/processor/LogUpdateProcessorFactory.java
##########
@@ -108,7 +108,9 @@ public void processAdd(AddUpdateCommand cmd) throws IOException {
// Add a list of added id's to the response
if (adds == null) {
adds = new ArrayList<>();
- toLog.add("add",adds);
+ synchronized (toLog) {
+ toLog.add("add",adds);
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `LogUpdateProcessorFactory$LogUpdateProcessor.processAdd(...)` reads with synchronization from `this.adds`. Potentially races with unsynchronized write in method `LogUpdateProcessorFactory$LogUpdateProcessor.processAdd(...)`.
Reporting because this access may occur on a background thread.
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/update/processor/LogUpdateProcessorFactory.java
##########
@@ -108,7 +108,9 @@ public void processAdd(AddUpdateCommand cmd) throws IOException {
// Add a list of added id's to the response
if (adds == null) {
adds = new ArrayList<>();
Review comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `LogUpdateProcessorFactory$LogUpdateProcessor.processAdd(...)` writes to field `this.adds` outside of synchronization.
Reporting because this access may occur on a background thread.
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/update/processor/LogUpdateProcessorFactory.java
##########
@@ -131,7 +133,9 @@ public void processDelete( DeleteUpdateCommand cmd ) throws IOException {
if (cmd.isDeleteById()) {
if (deletes == null) {
deletes = new ArrayList<>();
- toLog.add("delete",deletes);
+ synchronized (toLog) {
+ toLog.add("delete",deletes);
Review comment:
*THREAD_SAFETY_VIOLATION:* Read/Write race. Non-private method `LogUpdateProcessorFactory$LogUpdateProcessor.processDelete(...)` reads with synchronization from `this.deletes`. Potentially races with unsynchronized write in method `LogUpdateProcessorFactory$LogUpdateProcessor.processDelete(...)`.
Reporting because this access may occur on a background thread.
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/update/processor/LogUpdateProcessorFactory.java
##########
@@ -140,11 +144,13 @@ public void processDelete( DeleteUpdateCommand cmd ) throws IOException {
Review comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `LogUpdateProcessorFactory$LogUpdateProcessor.processDelete(...)` indirectly writes to field `cmd.id` outside of synchronization.
Reporting because this access may occur on a background thread.
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/update/processor/LogUpdateProcessorFactory.java
##########
@@ -131,7 +133,9 @@ public void processDelete( DeleteUpdateCommand cmd ) throws IOException {
if (cmd.isDeleteById()) {
if (deletes == null) {
deletes = new ArrayList<>();
Review comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `LogUpdateProcessorFactory$LogUpdateProcessor.processDelete(...)` writes to field `this.deletes` outside of synchronization.
Reporting because this access may occur on a background thread.
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)
##########
File path: solr/core/src/java/org/apache/solr/update/processor/LogUpdateProcessorFactory.java
##########
@@ -140,11 +144,13 @@ public void processDelete( DeleteUpdateCommand cmd ) throws IOException {
deletes.add(msg);
}
} else {
- if (toLog.size() < maxNumToLog) {
- long version = cmd.getVersion();
- String msg = cmd.query;
- if (version != 0) msg = msg + " (" + version + ')';
- toLog.add("deleteByQuery", msg);
+ synchronized (toLog) {
+ if (toLog.size() < maxNumToLog) {
+ long version = cmd.getVersion();
+ String msg = cmd.query;
+ if (version != 0) msg = msg + " (" + version + ')';
+ toLog.add("deleteByQuery", msg);
+ }
}
}
numDeletes++;
Review comment:
*THREAD_SAFETY_VIOLATION:* Unprotected write. Non-private method `LogUpdateProcessorFactory$LogUpdateProcessor.processDelete(...)` writes to field `this.numDeletes` outside of synchronization.
Reporting because this access may occur on a background thread.
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke closed pull request #597: SOLR-15983: synchronise LogUpdateProcessor's members use
Posted by GitBox <gi...@apache.org>.
cpoerschke closed pull request #597:
URL: https://github.com/apache/solr/pull/597
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on pull request #597: SOLR-15983: synchronise LogUpdateProcessor's members use
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on pull request #597:
URL: https://github.com/apache/solr/pull/597#issuecomment-1031654414
closing in favour of alternative changes e.g. #601 (or something else still)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org