GoogleCloudPlatform / jetty-runtime

Google Cloud Platform Jetty Docker image
Apache License 2.0
44 stars 33 forks source link

ContextDepth increment #262

Closed aleksandr-nesterov closed 5 years ago

aleksandr-nesterov commented 5 years ago

Looks like incrementation of thread-local contextDepth has a bug. In RequestContextScope thread-local value either becomes 1 or 2. Here is the code:

Integer depth = contextDepth.get();
if (depth == null || depth.intValue() == 0) {
    depth = 1;
    String currentTraceId = (String) request.getAttribute(X_CLOUD_TRACE);
    if (currentTraceId == null) {
           // ...
    } else {
        depth = depth + 1;
    }
    contextDepth.set(depth);
}

Shouldnt it be?

Integer depth = contextDepth.get();
if (depth == null || depth.intValue() == 0) {
    depth = 1;
    String currentTraceId = (String) request.getAttribute(X_CLOUD_TRACE);
    if (currentTraceId == null) {
           // ...
    } 
} else {
    depth = depth + 1;
}
contextDepth.set(depth);
gregw commented 5 years ago

@aleksandr-nesterov I believe you are right and that there is a bug in the balancing of the braces. The effect of this is that some threads may remain associated with long dead requests!

joakime commented 5 years ago

PR #264 was merged for this issue, can it be closed now?