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.

Sign up for GitHub

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

Jump to bottom

Another round of MSC fixes #51

Merged
merged 37 commits into from
Oct 30, 2017
Merged

Another round of MSC fixes #51

Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
87314ff
[MSC-170] Fixing startup lifecycleTime computation issue. Moving it t…
ropalka Feb 2, 2017
e7c9701
[MSC-181] Refactoring - no functional change.
ropalka May 24, 2017
a353add
[MSC-181] Refactoring - no functional change.
ropalka May 24, 2017
f95a5a9
[MSC-181] Refactoring - no functional change.
ropalka May 24, 2017
66af7c0
[MSC-180] Refactoring - no functional change.
ropalka Feb 8, 2017
c03c0ac
[MSC-180][MSC-167] Refactoring - no functional change. Splitting Depe…
ropalka Jan 26, 2017
432a97c
[MSC-180][MSC-168] Refactoring - no functional change. Splitting Stop…
ropalka Jan 26, 2017
2404a8c
[MSC-180][MSC-157] Refactoring - no functional change. Introducing ne…
ropalka Feb 2, 2017
4aad96a
Removing obsolete and now useless MSC benchmark tests.
ropalka Jun 5, 2017
f9e61a2
Removing all byteman based tests. There are two kinds of problems wit…
ropalka Jun 5, 2017
21da500
Refactoring - no functional change. Renaming ServiceUn/availableTask …
ropalka Jun 6, 2017
752f802
Refactoring - no functional change. Reordering inner classes.
ropalka Jun 6, 2017
d59ea2f
[MSC-182] Introducing DependenciesControllerTask base class.
ropalka Jun 6, 2017
0fe9f4c
[MSC-182] Refactoring - no functional change. Introducing new categor…
ropalka Jun 6, 2017
4fb8f48
[MSC-183] Introducing DependentsControllerTask base class.
ropalka Jun 6, 2017
8ad920b
[MSC-183] Refactoring - no functional change. Introducing new categor…
ropalka Jun 6, 2017
6b59975
[MSC-183] Refactoring - no functional change. Eliminating ServiceCont…
ropalka Jun 6, 2017
a954cac
[MSC-183] Refactoring - no functional change. Moving execute() to Dep…
ropalka Jun 6, 2017
05abd6f
Refactoring - no functional change. All ServiceControllerImpl inner c…
ropalka Jun 6, 2017
8ca9fe5
[MSC-179] partial revert of commit id a3d68c0a.
ropalka Jun 21, 2017
445d5c4
Refactoring - no functional change. Enforcing FAIL FAST best practices.
ropalka Jun 3, 2017
4a53b8b
Removing tests on OPTIONAL dependencies.
ropalka Jun 9, 2017
cccc4a5
[MSC-184] Completely fixing buggy optional dependency implementation.
ropalka Apr 26, 2017
6d517be
[MSC-184][MSC-185] Implementing 'dependents started counting' mechani…
ropalka Mar 24, 2017
29dbff7
[MSC-186] Since now on listener transition notifications
ropalka Feb 9, 2017
4fe0ccf
[MSC-187] Introducing 'dependent tasks execution completion detection…
ropalka Jun 7, 2017
5fca3e5
[MSC-187] Revisited and reimplemented ServiceRegistrationImpl locking…
ropalka Jun 7, 2017
83ab427
[MSC-188] Fixing controller locked forever in START_REQUESTED state i…
ropalka May 13, 2017
a0dd8e5
[MSC-189] Fixing controller async tasks count invariant breakage.
ropalka Jun 19, 2017
4e716bb
[MSC-190] Controller can create new tasks as response for incoming
ropalka Jun 20, 2017
55c56ec
[MSC-191] Always ensure all controller invariants
ropalka Mar 22, 2017
b0174a0
[MSC-192] Eliminating notion of 'transitive notifications'
ropalka Mar 16, 2017
aaa0cb0
Refactoring - no functional change.
ropalka Sep 5, 2017
6ecde00
Refactoring - no functional change.
ropalka Jun 9, 2017
becc235
Test that attempts to replicate the failure
stuartwdouglas Jan 5, 2017
5ef9dc0
More detailed logging on failure
bstansberry Jan 5, 2017
d07879a
[MSC-193] Fixing broken ServiceContainer shutdown process.
ropalka Sep 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
86 changes: 24 additions & 62 deletions src/main/java/org/jboss/msc/service/ServiceControllerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,12 +252,11 @@ void commitInstallation(Mode initialMode) {
synchronized (this) {
final boolean leavingRestState = isStableRestState();
tasks.add(new DependencyAvailableTask());
Dependent[][] dependents = getDependents();
if (!immediateUnavailableDependencies.isEmpty() || transitiveUnavailableDepCount > 0) {
tasks.add(new TransitiveDependencyUnavailableTask(dependents));
tasks.add(new TransitiveDependencyUnavailableTask());
}
if (failCount > 0) {
tasks.add(new DependencyFailedTask(dependents));
tasks.add(new DependencyFailedTask());
}
state = Substate.DOWN;
// subtract one to compensate for +1 above
Expand Down Expand Up @@ -571,12 +570,12 @@ private void transition(final ArrayList<Runnable> tasks) {
dependenciesDemanded = false;
}
getListenerTasks(transition, tasks);
tasks.add(new DependencyStoppedTask(getDependents()));
Copy link
Contributor

Choose a reason for hiding this comment

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

The fact that we're no longer using a snapshot of the dependents array is a functional change I think, because now we're iterating dependents outside of the lock.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is really no functional change @dmlloyd :)
The controller dependents are initialized in DependentsControllerTask constructor
and all tasks are always instantiated under intrinsic lock. See previous commit.

tasks.add(new DependencyStoppedTask());
break;
}
case STARTING_to_UP: {
getListenerTasks(transition, tasks);
tasks.add(new DependencyStartedTask(getDependents()));
tasks.add(new DependencyStartedTask());
break;
}
case STARTING_to_START_FAILED: {
Expand All @@ -590,7 +589,7 @@ private void transition(final ArrayList<Runnable> tasks) {
this.childTarget = null;
}
getListenerTasks(transition, tasks);
tasks.add(new DependencyFailedTask(getDependents()));
tasks.add(new DependencyFailedTask());
tasks.add(new RemoveChildrenTask());
break;
}
Expand All @@ -600,7 +599,7 @@ private void transition(final ArrayList<Runnable> tasks) {
monitor.removeFailed(this);
}
getListenerTasks(transition, tasks);
tasks.add(new DependencyRetryingTask(getDependents()));
tasks.add(new DependencyRetryingTask());
break;
}
case START_INITIATING_to_STARTING: {
Expand All @@ -616,14 +615,14 @@ private void transition(final ArrayList<Runnable> tasks) {
startException = null;
failCount--;
getListenerTasks(transition, tasks);
tasks.add(new DependencyRetryingTask(getDependents()));
tasks.add(new DependencyRetryingTask());
tasks.add(new StopTask(true));
tasks.add(new DependentStoppedTask());
break;
}
case STOP_REQUESTED_to_UP: {
getListenerTasks(transition, tasks);
tasks.add(new DependencyStartedTask(getDependents()));
tasks.add(new DependencyStartedTask());
break;
}
case STOP_REQUESTED_to_STOPPING: {
Expand All @@ -640,13 +639,12 @@ private void transition(final ArrayList<Runnable> tasks) {
case DOWN_to_REMOVING: {
getListenerTasks(transition, tasks);
tasks.add(new DependencyUnavailableTask());
Dependent[][] dependents = getDependents();
// Clear all dependency uninstalled flags from dependents
if (!immediateUnavailableDependencies.isEmpty() || transitiveUnavailableDepCount > 0) {
tasks.add(new TransitiveDependencyAvailableTask(dependents));
tasks.add(new TransitiveDependencyAvailableTask());
}
if (failCount > 0) {
tasks.add(new DependencyRetryingTask(dependents));
tasks.add(new DependencyRetryingTask());
}
break;
}
Expand Down Expand Up @@ -780,7 +778,7 @@ public void immediateDependencyAvailable(ServiceName dependencyName) {
// both unavailable dep counts are 0
if (transitiveUnavailableDepCount == 0) {
transition(tasks);
tasks.add(new TransitiveDependencyAvailableTask(getDependents()));
tasks.add(new TransitiveDependencyAvailableTask());
}
addAsyncTasks(tasks.size());
updateStabilityState(leavingRestState);
Expand All @@ -806,7 +804,7 @@ public void immediateDependencyUnavailable(ServiceName dependencyName) {
// otherwise, they have already been notified
if (transitiveUnavailableDepCount == 0) {
transition(tasks);
tasks.add(new TransitiveDependencyUnavailableTask(getDependents()));
tasks.add(new TransitiveDependencyUnavailableTask());
}
addAsyncTasks(tasks.size());
updateStabilityState(leavingRestState);
Expand All @@ -830,7 +828,7 @@ public void transitiveDependencyAvailable() {
// there are no immediate nor transitive unavailable dependencies
if (immediateUnavailableDependencies.isEmpty()) {
transition(tasks);
tasks.add(new TransitiveDependencyAvailableTask(getDependents()));
tasks.add(new TransitiveDependencyAvailableTask());
}
addAsyncTasks(tasks.size());
updateStabilityState(leavingRestState);
Expand All @@ -855,7 +853,7 @@ public void transitiveDependencyUnavailable() {
// otherwise, they have already been notified
if (immediateUnavailableDependencies.isEmpty()) {
transition(tasks);
tasks.add(new TransitiveDependencyUnavailableTask(getDependents()));
tasks.add(new TransitiveDependencyUnavailableTask());
}
addAsyncTasks(tasks.size());
updateStabilityState(leavingRestState);
Expand Down Expand Up @@ -915,7 +913,7 @@ public void dependencyFailed() {
if (state == Substate.PROBLEM) {
getListenerTasks(ListenerNotification.DEPENDENCY_FAILURE, tasks);
}
tasks.add(new DependencyFailedTask(getDependents()));
tasks.add(new DependencyFailedTask());
addAsyncTasks(tasks.size());
updateStabilityState(leavingRestState);
}
Expand All @@ -935,7 +933,7 @@ public void dependencyFailureCleared() {
if (state == Substate.PROBLEM) {
getListenerTasks(ListenerNotification.DEPENDENCY_FAILURE_CLEAR, tasks);
}
tasks.add(new DependencyRetryingTask(getDependents()));
tasks.add(new DependencyRetryingTask());
addAsyncTasks(tasks.size());
updateStabilityState(leavingRestState);
}
Expand Down Expand Up @@ -1649,7 +1647,7 @@ private class DependentStoppedTask extends DependenciesControllerTask {
void inform(final ServiceControllerImpl parent) { parent.dependentStopped(); }
}

private class DependencyAvailableTask extends ControllerTask {
private class DependencyAvailableTask extends DependentsControllerTask {
private final Map<ServiceName, Dependent[]> dependents;
private final Dependent[] children;

Expand All @@ -1673,7 +1671,7 @@ boolean execute() {
}
}

private class DependencyUnavailableTask extends ControllerTask {
private class DependencyUnavailableTask extends DependentsControllerTask {
private final Map<ServiceName, Dependent[]> dependents;
private final Dependent[] children;

Expand All @@ -1697,13 +1695,7 @@ boolean execute() {
}
}

private class DependencyStartedTask extends ControllerTask {
private final Dependent[][] dependents;

DependencyStartedTask(final Dependent[][] dependents) {
this.dependents = dependents;
}

private class DependencyStartedTask extends DependentsControllerTask {
boolean execute() {
for (Dependent[] dependentArray : dependents) {
for (Dependent dependent : dependentArray) {
Expand All @@ -1714,13 +1706,7 @@ boolean execute() {
}
}

private class DependencyStoppedTask extends ControllerTask {
private final Dependent[][] dependents;

DependencyStoppedTask(final Dependent[][] dependents) {
this.dependents = dependents;
}

private class DependencyStoppedTask extends DependentsControllerTask {
boolean execute() {
for (Dependent[] dependentArray : dependents) {
for (Dependent dependent : dependentArray) {
Expand All @@ -1731,13 +1717,7 @@ boolean execute() {
}
}

private class DependencyFailedTask extends ControllerTask {
private final Dependent[][] dependents;

DependencyFailedTask(final Dependent[][] dependents) {
this.dependents = dependents;
}

private class DependencyFailedTask extends DependentsControllerTask {
boolean execute() {
for (Dependent[] dependentArray : dependents) {
for (Dependent dependent : dependentArray) {
Expand All @@ -1748,13 +1728,7 @@ boolean execute() {
}
}

private class DependencyRetryingTask extends ControllerTask {
private final Dependent[][] dependents;

DependencyRetryingTask(final Dependent[][] dependents) {
this.dependents = dependents;
}

private class DependencyRetryingTask extends DependentsControllerTask {
boolean execute() {
for (Dependent[] dependentArray : dependents) {
for (Dependent dependent : dependentArray) {
Expand All @@ -1765,13 +1739,7 @@ boolean execute() {
}
}

private class TransitiveDependencyAvailableTask extends ControllerTask {
private final Dependent[][] dependents;

TransitiveDependencyAvailableTask(final Dependent[][] dependents) {
this.dependents = dependents;
}

private class TransitiveDependencyAvailableTask extends DependentsControllerTask {
boolean execute() {
for (Dependent[] dependentArray : dependents) {
for (Dependent dependent : dependentArray) {
Expand All @@ -1782,13 +1750,7 @@ boolean execute() {
}
}

private class TransitiveDependencyUnavailableTask extends ControllerTask {
private final Dependent[][] dependents;

TransitiveDependencyUnavailableTask(final Dependent[][] dependents) {
this.dependents = dependents;
}

private class TransitiveDependencyUnavailableTask extends DependentsControllerTask {
boolean execute() {
for (Dependent[] dependentArray : dependents) {
for (Dependent dependent : dependentArray) {
Expand Down