Skip to content

Commit c3387b6

Browse files
kohtalagrs
authored andcommitted
Cancel idle abortion timer
Receiving max idle time exceeded left abort_idle timer running when connection was already closed causing possibly second disconnected event. The CLOSE frame error was not cleared, but it was resent on reconnect after the client had send the timeout error.
1 parent 0555b48 commit c3387b6

File tree

4 files changed

+192
-7
lines changed

4 files changed

+192
-7
lines changed

lib/connection.js

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ var Connection = function (options, container) {
239239
this.conn_established_counter = 0;
240240
this.heartbeat_out = undefined;
241241
this.heartbeat_in = undefined;
242-
this.abort_idle = false;
242+
this.abort_idle = undefined;
243243
this.socket_ready = false;
244244
this.scheduled_reconnect = undefined;
245245
this.default_sender = undefined;
@@ -274,7 +274,8 @@ Connection.prototype._disconnect = function() {
274274

275275
Connection.prototype._reconnect = function() {
276276
if (this.abort_idle) {
277-
this.abort_idle = false;
277+
clearTimeout(this.abort_idle);
278+
this.abort_idle = undefined;
278279
this.local.close.error = undefined;
279280
this.state = new EndpointState();
280281
this.state.open();
@@ -302,7 +303,10 @@ Connection.prototype._reset_remote_state = function() {
302303

303304
Connection.prototype.connect = function () {
304305
this.is_server = false;
305-
this.abort_idle = false;
306+
if (this.abort_idle) {
307+
clearTimeout(this.abort_idle);
308+
this.abort_idle = undefined;
309+
}
306310
this._reset_remote_state();
307311
this._connect(this.options.connection_details(this.conn_established_counter));
308312
this.open();
@@ -341,6 +345,7 @@ Connection.prototype.accept = function (socket) {
341345

342346
Connection.prototype.abort_socket = function (socket) {
343347
if (socket === this.socket) {
348+
this.abort_idle = undefined;
344349
log.io('[%s] aborting socket', this.options.id);
345350
this.socket.end();
346351
if (this.socket.removeAllListeners) {
@@ -575,11 +580,10 @@ Connection.prototype.input = function (buff) {
575580

576581
Connection.prototype.idle = function () {
577582
if (!this.is_closed()) {
578-
this.abort_idle = true;
579583
this.closed_with_non_fatal_error = true;
580584
this.local.close.error = {condition:'amqp:resource-limit-exceeded', description:'max idle time exceeded'};
581585
this.close();
582-
setTimeout(this.abort_socket.bind(this, this.socket), 1000);
586+
this.abort_idle = setTimeout(this.abort_socket.bind(this, this.socket), 1000);
583587
}
584588
};
585589

@@ -602,6 +606,10 @@ Connection.prototype._disconnected = function (error) {
602606
clearTimeout(this.heartbeat_in);
603607
this.heartbeat_in = undefined;
604608
}
609+
if (this.abort_idle) {
610+
clearTimeout(this.abort_idle);
611+
this.abort_idle = undefined;
612+
}
605613
var was_closed_with_non_fatal_error = this.closed_with_non_fatal_error;
606614
if (this.closed_with_non_fatal_error) {
607615
this.closed_with_non_fatal_error = false;
@@ -756,6 +764,7 @@ Connection.prototype._write_open = function () {
756764

757765
Connection.prototype._write_close = function () {
758766
this._write_frame(0, this.local.close);
767+
this.local.close.error = undefined;
759768
};
760769

761770
Connection.prototype.on_begin = function (frame) {

package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,7 @@
2929
"ts-node": "^10.4.0",
3030
"typescript": "^4.5.5",
3131
"uglify-js": "",
32-
"ws": "^6.0.0",
33-
"wtfnode": "^0.8.4"
32+
"ws": "^6.0.0"
3433
},
3534
"scripts": {
3635
"lint": "eslint lib/*.js",

test/connections.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ describe('connection error handling', function () {
163163
var error_handler_called: boolean;
164164
var close_handler_called: boolean;
165165
container.on('connection_open', function (context: rhea.EventContext) {
166+
assert.strictEqual(context.connection.error, undefined);
166167
context.connection.close({ condition: 'amqp:connection:forced', description: 'testing error on close' });
167168
});
168169
container.on('connection_close', function (context: rhea.EventContext) {

test/idle.ts

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
/*
2+
* Copyright 2015 Red Hat Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import * as assert from "assert";
18+
import { AddressInfo, Server } from "net";
19+
import * as rhea from "../";
20+
21+
describe('idle', function () {
22+
var server: rhea.Container;
23+
var listener: Server;
24+
25+
beforeEach(function() {
26+
})
27+
28+
afterEach(function () {
29+
listener.close();
30+
});
31+
32+
describe('server has idle_time_out', function () {
33+
beforeEach(function (done: Function) {
34+
server = rhea.create_container({ non_fatal_errors: [] });
35+
listener = server.listen({ port: 0, idle_time_out: 300 });
36+
listener.on('listening', function () {
37+
done();
38+
});
39+
});
40+
41+
// There are setups where server detects idle due to suspend while
42+
// networking and all is well.
43+
it('client does not send within idle_time_out', function (done: Function) {
44+
var state = 0;
45+
server.on('connection_open', function (context: rhea.EventContext) {
46+
assert.strictEqual(++state, 1);
47+
});
48+
server.on('connection_error', function (context: rhea.EventContext) {
49+
assert.fail('server must not receive error');
50+
});
51+
server.on('connection_close', function (context: rhea.EventContext) {
52+
assert.strictEqual(++state, 4);
53+
});
54+
server.on('disconnected', function (context: rhea.EventContext) {
55+
assert.strictEqual(context.reconnecting, undefined);
56+
assert.strictEqual(++state, 5);
57+
setTimeout(() => { done() }, 100);
58+
});
59+
60+
var client: rhea.Container = rhea.create_container();
61+
var conn = client.connect({
62+
port: (listener.address() as AddressInfo).port
63+
});
64+
conn.on('connection_open', function (context: rhea.EventContext) {
65+
const connection = context.connection;
66+
assert.strictEqual(++state, 2);
67+
assert.strictEqual(connection.remote.open.idle_time_out, 300);
68+
// Disable idle timer
69+
connection.remote.open.idle_time_out = 0;
70+
if (connection.heartbeat_out) clearTimeout(connection.heartbeat_out);
71+
});
72+
conn.on('connection_error', function (context: rhea.EventContext) {
73+
assert.strictEqual(++state, 3);
74+
var error = context.connection.error;
75+
assert.strictEqual((error as any).condition, 'amqp:resource-limit-exceeded');
76+
assert.strictEqual((error as any).description, 'max idle time exceeded');
77+
});
78+
conn.on('disconnected', function (context: rhea.EventContext) {
79+
assert.fail('disconnected shouldnt have been called');
80+
});
81+
});
82+
});
83+
84+
describe('server has no idle_time_out', function () {
85+
beforeEach(function (done: Function) {
86+
server = rhea.create_container({ non_fatal_errors: [] });
87+
listener = server.listen({ port: 0 });
88+
listener.on('listening', function () {
89+
done();
90+
});
91+
});
92+
93+
it('server does not send within idle_time_out', function (done: Function) {
94+
var state = 0;
95+
var server_opens = 0;
96+
var server_closes = 0;
97+
server.on('connection_open', function (context: rhea.EventContext) {
98+
++server_opens;
99+
const connection = context.connection;
100+
assert.strictEqual(connection.remote.open.idle_time_out, 500);
101+
if (server_opens == 1) {
102+
// Disable idle timer
103+
assert.strictEqual(++state, 1);
104+
connection.remote.open.idle_time_out = 0;
105+
if (connection.heartbeat_out) clearTimeout(connection.heartbeat_out);
106+
} else {
107+
// Reconnect after failure. Just close.
108+
assert.strictEqual(++state, 7);
109+
connection.close({ condition: 'time to end this test', description: 'well done' })
110+
}
111+
});
112+
server.on('connection_error', function (context: rhea.EventContext) {
113+
var error = context.connection.error;
114+
assert.strictEqual((error as any).condition, 'amqp:resource-limit-exceeded');
115+
assert.strictEqual((error as any).description, 'max idle time exceeded');
116+
assert.strictEqual(++state, 3);
117+
});
118+
server.on('connection_close', function (context: rhea.EventContext) {
119+
++server_closes;
120+
if (server_closes == 1) {
121+
assert.strictEqual(++state, 4);
122+
} else {
123+
done();
124+
}
125+
});
126+
server.on('disconnected', function (context: rhea.EventContext) {
127+
assert.fail('disconnected shouldnt have been called');
128+
});
129+
130+
var client: rhea.Container = rhea.create_container();
131+
var conn = client.connect({
132+
port: (listener.address() as AddressInfo).port,
133+
idle_time_out: 500
134+
});
135+
var client_opens = 0;
136+
var client_closes = 0;
137+
var client_disconnects = 0;
138+
conn.on('connection_open', function (context: rhea.EventContext) {
139+
client_opens++;
140+
assert.strictEqual(client_opens, server_opens);
141+
if (client_opens === 2) {
142+
assert.strictEqual(++state, 8);
143+
conn.close();
144+
} else {
145+
assert.strictEqual(++state, 2);
146+
}
147+
});
148+
conn.on('connection_error', function (context: rhea.EventContext) {
149+
var error = context.connection.error;
150+
assert.strictEqual((error as any).condition, 'time to end this test');
151+
assert.strictEqual(++state, 9);
152+
});
153+
conn.on('connection_close', function (context: rhea.EventContext) {
154+
client_closes++;
155+
if (client_closes == 1) {
156+
assert.strictEqual(client_closes, 1);
157+
assert.strictEqual(server_opens, 1);
158+
assert.strictEqual(client_opens, 1);
159+
assert.strictEqual(++state, 5);
160+
} else {
161+
assert.strictEqual(client_closes, 2);
162+
assert.strictEqual(server_opens, 2);
163+
assert.strictEqual(client_opens, 2);
164+
assert.strictEqual(++state, 10);
165+
}
166+
});
167+
conn.on('disconnected', function (context: rhea.EventContext) {
168+
client_disconnects++;
169+
assert.strictEqual(client_disconnects, 1);
170+
assert.strictEqual(server_opens, 1);
171+
assert.strictEqual(context.reconnecting, true);
172+
assert.strictEqual(++state, 6);
173+
});
174+
});
175+
});
176+
});

0 commit comments

Comments
 (0)