Re: [PR] ARTEMIS-4630 Validate if the pid is really from Artemis [activemq-artemis]
jbertram commented on PR #4811: URL: https://github.com/apache/activemq-artemis/pull/4811#issuecomment-1930411947 These commits have been reverted. @angelogalvao, I think if we want to address this problem we need to determine _how_ the wrong pid made it into `artemis.pid` in the first place. -- 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: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] ARTEMIS-4630 Validate if the pid is really from Artemis [activemq-artemis]
gemmellr commented on PR #4811: URL: https://github.com/apache/activemq-artemis/pull/4811#issuecomment-1930180134 (I actually only cherry-picked the second commit earlier, I thought the first was the same as yesterdays.) Reverting seems reasonable based on what you just said. FWIW past 'check the process' stuff I have seen tended to just add something as a marker to the command run originally, so it could be looked for later. -- 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: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] ARTEMIS-4630 Validate if the pid is really from Artemis [activemq-artemis]
jbertram commented on PR #4811: URL: https://github.com/apache/activemq-artemis/pull/4811#issuecomment-1930048938 After further investigation I don't think this solution is valid. The problem is that `lsof -a -d cwd -p ` will report the directory from which the `artemis-service` script _was executed_. However, this script can be invoked from *any* directory. Therefore, it's very easy to break the script with this change such that it actually starts the broker but reports that it didn't and then continues to report the wrong status. I will revert these commits soon. -- 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: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] ARTEMIS-4630 Validate if the pid is really from Artemis [activemq-artemis]
gemmellr commented on PR #4811: URL: https://github.com/apache/activemq-artemis/pull/4811#issuecomment-1929623659 I cherry-picked the fix and pushed it since the PR had conflicts. -- 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: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] ARTEMIS-4630 Validate if the pid is really from Artemis [activemq-artemis]
asfgit closed pull request #4811: ARTEMIS-4630 Validate if the pid is really from Artemis URL: https://github.com/apache/activemq-artemis/pull/4811 -- 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: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] ARTEMIS-4630 Validate if the pid is really from Artemis [activemq-artemis]
jbertram commented on PR #4801: URL: https://github.com/apache/activemq-artemis/pull/4801#issuecomment-1928102101 @angelogalvao, nice work. Thanks for the contribution! -- 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: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] ARTEMIS-4630 Validate if the pid is really from Artemis [activemq-artemis]
jbertram merged PR #4801: URL: https://github.com/apache/activemq-artemis/pull/4801 -- 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: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] ARTEMIS-4630 Validate if the pid is really from Artemis [activemq-artemis]
jbertram commented on code in PR #4801:
URL: https://github.com/apache/activemq-artemis/pull/4801#discussion_r1476928352
##
artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/bin/artemis-service:
##
@@ -61,7 +61,7 @@ status() {
pid=`cat "${PID_FILE}"`
# check to see if it's gone...
ps -p ${pid} > /dev/null
-if [ $? -eq 0 ] ; then
+if [ $? -eq 0 ] && [ "`pwdx ${pid} | awk '{print $2}'`" ==
"$ARTEMIS_INSTANCE" ]; then
Review Comment:
Unfortunately it's still broken. I still get the same error as before.
You're using `==` when I believe you should be using `=`.
--
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: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] ARTEMIS-4630 Validate if the pid is really from Artemis [activemq-artemis]
jbertram commented on code in PR #4801:
URL: https://github.com/apache/activemq-artemis/pull/4801#discussion_r1476733634
##
artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/bin/artemis-service:
##
@@ -61,7 +61,7 @@ status() {
pid=`cat "${PID_FILE}"`
# check to see if it's gone...
ps -p ${pid} > /dev/null
-if [ $? -eq 0 ] ; then
+if [ $? -eq 0 ] && [ `pwdx ${pid} | awk '{print $2}'` ==
"$ARTEMIS_INSTANCE" ]; then
Review Comment:
I couldn't get this to work in my testing. I kept getting this:
```
./artemis-service: 64: [: /home/jbertram: unexpected operator
```
However, this worked:
```
if [ $? -eq 0 ] && [ "`pwdx ${pid} | awk '{print $2}'`" =
"$ARTEMIS_INSTANCE" ] ; then
```
--
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: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] ARTEMIS-4630 Validate if the pid is really from Artemis [activemq-artemis]
jbertram commented on PR #4801: URL: https://github.com/apache/activemq-artemis/pull/4801#issuecomment-1924427040 Do you have any idea what leads to this situation with the pid in the first place? -- 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: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
