Skip to content

Commit 8844d87

Browse files
RealDolosRealDolos
authored
Fix OOM due to update avalanches (#7)
1 parent 1c00194 commit 8844d87

File tree

4 files changed

+106
-93
lines changed

4 files changed

+106
-93
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,9 @@ Internal class used to automatically update databases, exposed to allow advanced
136136
- `update` Emitted when new databases have been written to the filesystem.
137137
- **Methods**
138138
- *checkUpdates()* Checks for new database updates and downloads them.
139-
- *triggerUpdate()* Replaces the current databases with fresh copies from the mirror.
140139
- *close()* Shuts down the updater.
141140
- **Props**
141+
- `checking`: `<boolean>` Whether databases are being checked for updates in the background right now - see `checking` event.
142142
- `downloading`: `<boolean>` Whether databases are being downloaded in the background right now - see `downloading` event.
143143

144144
## Warning

‎index.js‎

Lines changed: 64 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ class UpdateSubscriber extends EventEmitter {
1919
super();
2020

2121
this.downloading = false;
22+
this.checking = false;
2223

2324
this._checker = setInterval(() => {
2425
this.checkUpdates();
@@ -34,17 +35,35 @@ class UpdateSubscriber extends EventEmitter {
3435
}
3536

3637
async checkUpdates() {
37-
// Leave time for listeners to be up
38-
await this._wait(200);
39-
40-
this.emit('checking');
41-
downloadHelper.fetchChecksums().then(() => {
42-
return downloadHelper.verifyAllChecksums(downloadPath);
43-
}).then(() => {
44-
// All checksums match, no update
45-
}).catch(() => {
46-
this.update();
47-
});
38+
if (this.checking) {
39+
return;
40+
}
41+
42+
this.checking = true;
43+
44+
try {
45+
// Leave time for listeners to be up
46+
await this._wait(200);
47+
48+
this.emit('checking');
49+
try {
50+
await downloadHelper.fetchChecksums();
51+
return await downloadHelper.verifyAllChecksums(downloadPath);
52+
}
53+
catch (ex) {
54+
this.update();
55+
};
56+
}
57+
finally {
58+
this.checking = false;
59+
this.emit('done checking');
60+
}
61+
}
62+
63+
// Backward compat
64+
triggerUpdate() {
65+
console.warn('geolite2-redist: triggerUpdate() is deprecated');
66+
return Promise.resolve();
4867
}
4968

5069
update() {
@@ -75,18 +94,6 @@ class UpdateSubscriber extends EventEmitter {
7594
});
7695
}
7796

78-
async triggerUpdate() {
79-
await this._wait(100);
80-
81-
downloadHelper.fetchChecksums().then(() => {
82-
return downloadHelper.verifyAllChecksums(downloadPath);
83-
}).then(() => {
84-
this.update();
85-
}).catch(error => {
86-
console.warn('geolite2 self-update error:', error);
87-
});
88-
}
89-
9097
close() {
9198
clearInterval(this._checker);
9299
}
@@ -99,55 +106,48 @@ class UpdateSubscriber extends EventEmitter {
99106
}
100107

101108
function wrapReader(reader, readerBuilder, db) {
102-
const proxyObject = {
103-
reader,
104-
database: db,
105-
lastUpdateCheck: 0,
106-
subscriber: null
107-
};
108-
109-
return new Proxy(proxyObject, {
110-
get: (proxyObject, prop) => {
111-
if (!proxyObject.subscriber) {
112-
proxyObject.subscriber = new UpdateSubscriber();
113-
114-
proxyObject.subscriber.on('update', async () => {
115-
proxyObject.reader = await readerBuilder(paths[proxyObject.database]);
116-
});
109+
const subscriber = new UpdateSubscriber();
110+
subscriber.on('update', async () => {
111+
reader = await readerBuilder(paths[db]);
112+
});
117113

118-
proxyObject.subscriber.on('checking', () => {
119-
proxyObject.lastUpdateCheck = Date.now();
120-
});
121-
}
114+
return new Proxy({}, {
115+
get: (_, prop) => {
116+
switch (prop) {
117+
case '_geolite2_triggerUpdateCheck':
118+
subscriber.checkUpdates();
119+
return 'OK';
122120

123-
if (Date.now() - proxyObject.lastUpdateCheck > updateTimer || prop === '_geolite2_triggerUpdateCheck') {
124-
proxyObject.subscriber.checkUpdates();
125-
if (prop === '_geolite2_triggerUpdateCheck') {
121+
case '_geolite2_triggerUpdate':
122+
// Keep this in for backward compat
126123
return 'OK';
127-
}
128-
}
129124

130-
if (prop === '_geolite2_triggerUpdate') {
131-
proxyObject.subscriber.triggerUpdate();
132-
return 'OK';
133-
}
125+
case '_geolite2_subscriber':
126+
return subscriber;
134127

135-
if (prop === '_geolite2_subscriber') {
136-
return proxyObject.subscriber;
137-
}
128+
case 'close':
129+
return () => {
130+
subscriber.close();
131+
reader = {};
132+
return true;
133+
};
138134

139-
if (prop === 'close') {
140-
proxyObject.subscriber.close();
141-
proxyObject.reader = {};
142-
return () => {
143-
return true;
144-
};
135+
default:
136+
return reader[prop];
145137
}
146-
147-
return proxyObject.reader[prop];
148138
},
149-
set: (proxyObject, prop, value) => {
150-
proxyObject.reader[prop] = value;
139+
set: (_, prop, value) => {
140+
switch (prop) {
141+
case '_geolite2_triggerUpdateCheck':
142+
case '_geolite2_triggerUpdate':
143+
case '_geolite2_subscriber':
144+
case 'close':
145+
throw new Error('Invalid property setter');
146+
147+
default:
148+
reader[prop] = value;
149+
break;
150+
}
151151
}
152152
});
153153
}

‎scripts/download-helper.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,9 @@ function verifyAllChecksums(downloadPath) {
9999
const promises = editions.map(edition => {
100100
return new Promise((resolve, reject) => {
101101
fs.readFile(path.join(downloadPath, edition.name + '.mmdb'), (err, buffer) => {
102-
if (err) {
102+
if (err || typeof buffer === 'undefined') {
103103
reject(err);
104+
return;
104105
}
105106

106107
const checksum = crypto.createHash('sha384').update(buffer, 'binary', 'hex').digest('hex');

‎test/index.js‎

Lines changed: 39 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
const assert = require('assert');
22
const fs = require('fs');
33
const geolite2 = require('../');
4+
const rimraf = require('rimraf');
45
const maxmind = require('maxmind');
56

67
describe('geolite2', function() {
@@ -41,9 +42,11 @@ describe('geolite2.UpdateSubscriber', function() {
4142
describe('#checkUpdates()', function() {
4243
it('should be able to check updates again', function() {
4344
return new Promise((resolve, reject) => {
44-
updateSubscriber.checkUpdates();
45-
updateSubscriber.once('checking', () => {
46-
resolve();
45+
updateSubscriber.once('done checking', () => {
46+
updateSubscriber.checkUpdates();
47+
updateSubscriber.once('checking', () => {
48+
resolve();
49+
});
4750
});
4851
});
4952
});
@@ -60,10 +63,15 @@ describe('geolite2.UpdateSubscriber', function() {
6063
});
6164
} else {
6265
// Trigger download
63-
updateSubscriber.triggerUpdate();
64-
updateSubscriber.once('downloading', () => {
65-
updateSubscriber.once('update', () => {
66-
resolve();
66+
updateSubscriber.once('done checking', () => {
67+
rimraf(require('path').resolve(__dirname, '../dbs'), e => {
68+
if (e) throw(e);
69+
updateSubscriber.checkUpdates();
70+
updateSubscriber.once('downloading', () => {
71+
updateSubscriber.once('update', () => {
72+
resolve();
73+
});
74+
});
6775
});
6876
});
6977
}
@@ -123,17 +131,19 @@ describe('geolite2.open', function() {
123131

124132
lookup._test_reader_replacement = 'control';
125133

126-
assert(lookup._geolite2_triggerUpdate === 'OK');
127-
128134
return new Promise((resolve, reject) => {
129-
lookup._geolite2_subscriber.once('update', () => {
130-
setTimeout(() => {
131-
if (lookup._test_reader_replacement === 'control') {
132-
reject(new Error('Reader instance wasn\'t updated'));
133-
} else {
134-
resolve();
135-
}
136-
}, 500);
135+
rimraf(require('path').resolve(__dirname, '../dbs'), e => {
136+
if (e) throw(e);
137+
assert(lookup._geolite2_triggerUpdateCheck === 'OK');
138+
lookup._geolite2_subscriber.once('update', () => {
139+
setTimeout(() => {
140+
if (lookup._test_reader_replacement === 'control') {
141+
reject(new Error('Reader instance wasn\'t updated'));
142+
} else {
143+
resolve();
144+
}
145+
}, 500);
146+
});
137147
});
138148
});
139149
});
@@ -175,17 +185,19 @@ describe('geolite2.open', function() {
175185

176186
lookup._test_reader_replacement = 'control';
177187

178-
assert(lookup._geolite2_triggerUpdate === 'OK');
179-
180188
return new Promise((resolve, reject) => {
181-
lookup._geolite2_subscriber.once('update', () => {
182-
setTimeout(() => {
183-
if (lookup._test_reader_replacement === 'control') {
184-
reject(new Error('Reader instance wasn\'t updated'));
185-
} else {
186-
resolve();
187-
}
188-
}, 500);
189+
rimraf(require('path').resolve(__dirname, '../dbs'), e => {
190+
if (e) throw(e);
191+
assert(lookup._geolite2_triggerUpdateCheck === 'OK');
192+
lookup._geolite2_subscriber.once('update', () => {
193+
setTimeout(() => {
194+
if (lookup._test_reader_replacement === 'control') {
195+
reject(new Error('Reader instance wasn\'t updated'));
196+
} else {
197+
resolve();
198+
}
199+
}, 500);
200+
});
189201
});
190202
});
191203
});

0 commit comments

Comments
 (0)