Skip to content
This repository was archived by the owner on Apr 19, 2026. It is now read-only.

Commit 308f522

Browse files
authored
Bugfix: data race in Service Control Client package (#64)
* Avoid data races leading to uninformative log entries. A data race between flushAndSchedule* methods and stop() causes a NullPointerException. The exception itself does not affect the operation of the library, but generates misleading log entries. Context: Under normal condiions flushAndSchedule* methods repeatedly schedule their own execution at constant time intervals. This way the library achieves regular execution of flushes. The execution should stop when the client is stopped. To achieve this, flushAndSchedule* methods reset counters and return if they detect a stopped client. Race condition: Client.stop() method might be called in one thread, while flushAndSchedule* are active. The method stop resets the field scheduler which in a while is dereferenced at the end of flushAndSchedule* methods. Solution: In flushAndSchedule* methods, check if the client is stopped for the second time towards the end of the function, just before dereferencing the field scheduler. Also make a local copy of the field scheduler. * Bump version to 1.0.14
1 parent aef637d commit 308f522

2 files changed

Lines changed: 22 additions & 4 deletions

File tree

endpoints-control/src/main/java/com/google/api/control/Client.java

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,13 @@ private void flushAndScheduleChecks() {
354354
log.atSevere().withCause(e).log("direct send of a check request %s failed", req);
355355
}
356356
}
357-
scheduler.enter(new Runnable() {
357+
// copy scheduler into a local variable to avoid data races beween this method and stop()
358+
Scheduler currentScheduler = scheduler;
359+
if (resetIfStopped()) {
360+
log.atFine().log("did not schedule succeeding check flush: client is stopped");
361+
return;
362+
}
363+
currentScheduler.enter(new Runnable() {
358364
@Override
359365
public void run() {
360366
flushAndScheduleChecks(); // Do this again after the interval
@@ -394,7 +400,13 @@ private void flushAndScheduleReports() {
394400
stop();
395401
return;
396402
}
397-
scheduler.enter(new Runnable() {
403+
// copy scheduler into a local variable to avoid data races beween this method and stop()
404+
Scheduler currentScheduler = scheduler;
405+
if (resetIfStopped()) {
406+
log.atFine().log("did not schedule succeeding report flush: client is stopped");
407+
return;
408+
}
409+
currentScheduler.enter(new Runnable() {
398410
@Override
399411
public void run() {
400412
flushAndScheduleReports(); // Do this again after the interval
@@ -435,7 +447,13 @@ private void flushAndScheduleQuota() {
435447
log.atSevere().withCause(e).log("direct send of a quota request %s failed", req);
436448
}
437449
}
438-
scheduler.enter(new Runnable() {
450+
// copy scheduler into a local variable to avoid data races beween this method and stop()
451+
Scheduler currentScheduler = scheduler;
452+
if (resetIfStopped()) {
453+
log.atFine().log("did not schedule succeeding quota flush: client is stopped");
454+
return;
455+
}
456+
currentScheduler.enter(new Runnable() {
439457
@Override
440458
public void run() {
441459
flushAndScheduleQuota(); // Do this again after the interval

gradle.properties

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
sourceCompatibility=1.7
1818
targetCompatibility=1.7
1919

20-
version = 1.0.13
20+
version = 1.0.14
2121
appengineSdkVersion = 1.9.56
2222
autoValueVersion = 1.6.6
2323
bouncycastleVersion = 1.54

0 commit comments

Comments
 (0)