Skip to content

Commit

Permalink
reset the client value of LazyConnectExchangeClient after close (#8881)
Browse files Browse the repository at this point in the history
  • Loading branch information
zrlw authored Sep 26, 2021
1 parent 35c2d7e commit d5ca09f
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ public boolean isClosed() {

@Override
public void close() {
if (closed) {
return;
}
closed = true;
try {
// graceful close
DefaultFuture.closeChannel(channel);
Expand All @@ -162,7 +166,6 @@ public void close(int timeout) {
if (closed) {
return;
}
closed = true;
if (timeout > 0) {
long start = System.currentTimeMillis();
while (DefaultFuture.hasFuture(channel)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,15 @@ public boolean isClosed() {
public void close() {
if (client != null) {
client.close();
client = null;
}
}

@Override
public void close(int timeout) {
if (client != null) {
client.close(timeout);
client = null;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ private void replaceWithLazyClient() {
/**
* the order of judgment in the if statement cannot be changed.
*/
if (!(client instanceof LazyConnectExchangeClient) || client.isClosed()) {
if (!(client instanceof LazyConnectExchangeClient)) {
// this is a defensive operation to avoid client is closed by accident, the initial state of the client is false
URL lazyUrl = url.addParameter(LAZY_CONNECT_INITIAL_STATE_KEY, Boolean.TRUE)
//.addParameter(RECONNECT_KEY, Boolean.FALSE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,11 @@ public void test_counter_error() {
Assertions.fail();
}

// client has been replaced with lazy client, close status is false because a new lazy client's exchange client is null.
Assertions.assertFalse(client.isClosed(), "client status close");
// invoker status is available because the default value of associated lazy client's initial state is true.
Assertions.assertTrue(helloServiceInvoker.isAvailable(), "invoker status unavailable");

// due to the effect of LazyConnectExchangeClient, the client will be "revived" whenever there is a call.
Assertions.assertEquals("hello", helloService.hello());
Assertions.assertEquals(1, LogUtil.findMessage(errorMsg), "should warning message");
Expand All @@ -184,9 +189,6 @@ public void test_counter_error() {

DubboAppender.doStop();

// status switch to available once invoke again
Assertions.assertTrue(helloServiceInvoker.isAvailable(), "client status available");

/**
* This is the third time to close the same client. Under normal circumstances,
* a client value should be closed once (that is, the shutdown operation is irreversible).
Expand All @@ -198,10 +200,14 @@ public void test_counter_error() {
*/
client.close();

// client has been replaced with lazy client. lazy client is fetched from referenceclientmap, and since it's
// been invoked once, it's close status is false
// close status is false because the lazy client's exchange client is null again after close().
Assertions.assertFalse(client.isClosed(), "client status close");
Assertions.assertFalse(helloServiceInvoker.isAvailable(), "client status close");
// invoker status is available because the default value of associated lazy client's initial state is true.
Assertions.assertTrue(helloServiceInvoker.isAvailable(), "invoker status unavailable");

// revive: initial the lazy client's exchange client again.
Assertions.assertEquals("hello", helloService.hello());

destoy();
}

Expand Down

0 comments on commit d5ca09f

Please sign in to comment.