You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/07/27 18:44:00 UTC
[GitHub] [maven-shared-utils] slawekjaranowski opened a new pull request, #113: [MSHARED-1072] fix blocking in StreamFeeder
slawekjaranowski opened a new pull request, #113:
URL: https://github.com/apache/maven-shared-utils/pull/113
If input stream has no more available data
StreamFeeder was block forever
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shared-utils] michael-o commented on pull request #113: [MSHARED-1072] fix blocking in StreamFeeder
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #113:
URL: https://github.com/apache/maven-shared-utils/pull/113#issuecomment-1516826066
@slawekjaranowski Do you want to pick this up? I want to relase Maven Release with this fix.
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shared-utils] slawekjaranowski commented on pull request #113: [MSHARED-1072] fix blocking in StreamFeeder
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #113:
URL: https://github.com/apache/maven-shared-utils/pull/113#issuecomment-1518191394
Tested with m-release-p ...
When we want to upgrade version in m-release-p we need #138
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shared-utils] slawekjaranowski commented on a diff in pull request #113: [MSHARED-1072] fix blocking in StreamFeeder
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #113:
URL: https://github.com/apache/maven-shared-utils/pull/113#discussion_r931481507
##########
src/main/java/org/apache/maven/shared/utils/cli/CommandLineTimeOutException.java:
##########
@@ -41,4 +40,11 @@ public CommandLineTimeOutException( String message, Throwable cause )
super( message, cause );
}
+ /**
+ * @param message The message of the exception.
+ */
+ public CommandLineTimeOutException( String message )
Review Comment:
Existing exception name ... I can rename .. but will be break, maybe ok for 4.x 😄
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shared-utils] slawekjaranowski merged pull request #113: [MSHARED-1072] fix blocking in StreamFeeder
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski merged PR #113:
URL: https://github.com/apache/maven-shared-utils/pull/113
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shared-utils] kwin commented on pull request #113: [MSHARED-1072] fix blocking in StreamFeeder
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on PR #113:
URL: https://github.com/apache/maven-shared-utils/pull/113#issuecomment-1487390021
@slawekjaranowski Given that maven-shared-utils is mostly dead, I am wondering whether you can provide the fix instead for https://github.com/codehaus-plexus/plexus-utils/tree/master/src/main/java/org/codehaus/plexus/util/cli and afterwards we convert maven-gpg-plugin to using plexus-utils again instead of maven-shared-utils.
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shared-utils] michael-o commented on a diff in pull request #113: [MSHARED-1072] fix blocking in StreamFeeder
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #113:
URL: https://github.com/apache/maven-shared-utils/pull/113#discussion_r931476426
##########
src/main/java/org/apache/maven/shared/utils/cli/CommandLineUtils.java:
##########
@@ -288,31 +279,13 @@ public Integer call()
errorPumper.setName( "StreamPumper-systemErr" );
errorPumper.start();
- int returnValue;
- if ( timeoutInSeconds <= 0 )
+ if ( timeoutInSeconds > 0 && !p.waitFor( timeoutInSeconds, TimeUnit.SECONDS ) )
Review Comment:
Where is the kill for this process? It may continue to run as a zombie forever.
##########
src/main/java/org/apache/maven/shared/utils/cli/CommandLineTimeOutException.java:
##########
@@ -41,4 +40,11 @@ public CommandLineTimeOutException( String message, Throwable cause )
super( message, cause );
}
+ /**
+ * @param message The message of the exception.
+ */
+ public CommandLineTimeOutException( String message )
Review Comment:
`Timeout` according to https://www.oxfordlearnersdictionaries.com/definition/english/timeout?q=timeout
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shared-utils] michael-o commented on a diff in pull request #113: [MSHARED-1072] fix blocking in StreamFeeder
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #113:
URL: https://github.com/apache/maven-shared-utils/pull/113#discussion_r1173486316
##########
src/main/java/org/apache/maven/shared/utils/cli/CommandLineUtils.java:
##########
@@ -436,7 +387,7 @@ public static String[] translateCommandline(String toProcess) throws CommandLine
boolean inEscape = false;
int state = normal;
final StringTokenizer tok = new StringTokenizer(toProcess, "\"\' \\", true);
Review Comment:
Off topic: This is sooo ugly...
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shared-utils] slawekjaranowski commented on pull request #113: [MSHARED-1072] fix blocking in StreamFeeder
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #113:
URL: https://github.com/apache/maven-shared-utils/pull/113#issuecomment-1487577938
> > @slawekjaranowski Given that maven-shared-utils is mostly dead, I am wondering whether you can provide the fix instead for https://github.com/codehaus-plexus/plexus-utils/tree/master/src/main/java/org/codehaus/plexus/util/cli and afterwards we convert maven-gpg-plugin to using plexus-utils again instead of maven-shared-utils.
>
> I think this fix deserves a last patch release and done.
Ok, I will rebase and check again 😄
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shared-utils] michael-o commented on a diff in pull request #113: [MSHARED-1072] fix blocking in StreamFeeder
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #113:
URL: https://github.com/apache/maven-shared-utils/pull/113#discussion_r931496218
##########
src/main/java/org/apache/maven/shared/utils/cli/CommandLineTimeOutException.java:
##########
@@ -41,4 +40,11 @@ public CommandLineTimeOutException( String message, Throwable cause )
super( message, cause );
}
+ /**
+ * @param message The message of the exception.
+ */
+ public CommandLineTimeOutException( String message )
Review Comment:
No, leave it
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shared-utils] famod commented on pull request #113: [MSHARED-1072] fix blocking in StreamFeeder
Posted by "famod (via GitHub)" <gi...@apache.org>.
famod commented on PR #113:
URL: https://github.com/apache/maven-shared-utils/pull/113#issuecomment-1474366436
Coming from https://issues.apache.org/jira/browse/MRELEASE-1114, what's the status of this? Are there any blockers?
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shared-utils] slawekjaranowski commented on a diff in pull request #113: [MSHARED-1072] fix blocking in StreamFeeder
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #113:
URL: https://github.com/apache/maven-shared-utils/pull/113#discussion_r931495170
##########
src/main/java/org/apache/maven/shared/utils/cli/CommandLineUtils.java:
##########
@@ -288,31 +279,13 @@ public Integer call()
errorPumper.setName( "StreamPumper-systemErr" );
errorPumper.start();
- int returnValue;
- if ( timeoutInSeconds <= 0 )
+ if ( timeoutInSeconds > 0 && !p.waitFor( timeoutInSeconds, TimeUnit.SECONDS ) )
Review Comment:
There is defined `ShutDownHook` lines 237 - 253, and it is also called in line 362
There are also `pumper` threads which consume all outputs of process.
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shared-utils] michael-o commented on pull request #113: [MSHARED-1072] fix blocking in StreamFeeder
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #113:
URL: https://github.com/apache/maven-shared-utils/pull/113#issuecomment-1487515994
> @slawekjaranowski Given that maven-shared-utils is mostly dead, I am wondering whether you can provide the fix instead for https://github.com/codehaus-plexus/plexus-utils/tree/master/src/main/java/org/codehaus/plexus/util/cli and afterwards we convert maven-gpg-plugin to using plexus-utils again instead of maven-shared-utils.
I think this fix deserves a last patch release and 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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shared-utils] slawekjaranowski commented on pull request #113: [MSHARED-1072] fix blocking in StreamFeeder
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #113:
URL: https://github.com/apache/maven-shared-utils/pull/113#issuecomment-1516939873
@michael-o refresh with current master, conflict resolved
Tomorrow I would to check again.
--
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@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org