Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NETBEANS-6177] Fix an issue the stop command is not sent #3549

Merged
merged 1 commit into from Feb 2, 2022

Conversation

junichi11
Copy link
Member

@junichi11 junichi11 added the PHP [ci] enable extra PHP tests (php/php.editor) label Feb 2, 2022
@junichi11 junichi11 added this to the NB13 milestone Feb 2, 2022
@junichi11
Copy link
Member Author

@neilcsmith-net Can we merge this into the delivery branch? If it's too late, let's merge it in NB14.

@@ -124,6 +125,14 @@ public void run() {
} catch (Throwable e) {
log(e, Level.SEVERE);
}
if (canceled) {
synchronized (commands) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks good to me. However, the threading in this class is not documented and is not clear to me - I can see no synchronization of commands in preprocess() method, for example.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tmysik
Copy link
Member

tmysik commented Feb 2, 2022

@neilcsmith-net please merge it if it is OK for you.

Thank you, guys!

@junichi11
Copy link
Member Author

@tmysik Thank you for your review!

- https://issues.apache.org/jira/browse/NETBEANS-6177
- Related to NETBEANS-5080
- Use the `canceled` field instead of `detachRequest.set(true)` because
the stop command is not sent in `sendStopCommand()` if `detacheRequest` is `true`
@neilcsmith-net
Copy link
Member

OK, thanks, just caught in time for syncing for 13-rc3 so will merge. Not sure if we'll have a 13-rc4 yet.

@neilcsmith-net neilcsmith-net merged commit 3ff1e3d into apache:delivery Feb 2, 2022
@junichi11 junichi11 deleted the netbeans-6177-xdebug branch February 2, 2022 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PHP [ci] enable extra PHP tests (php/php.editor)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants