opendev/gerrit
Code Issues Proposed changes

Eagerly evaluate Providers in GuiceServletScopePropagator

Browse Source 
In ReceiveCommits, the RequestPropagator is constructed in the main
thread, and then used from within the worker thread to create another
nested scope for the email sending task. If we defer getting from the
Providers until wrapImpl() is called in the worker thread, then
HttpCurrentUserProvider.get() is reevaluated in that thread, which
fails since there is no HttpServletRequest/Response available.

Instead, save the actual provided objects in
GuiceServletScopePropagator so references to the same objects are
available from any thread. This is safe because we explicitly only
propagate objects we know to be threadsafe, and if they're safe to
access from two threads, they should be safe from an arbitrary number.

This should also work for nesting scopes; when a new
GuiceServletScopePropagator is required, it will inject CurrentUser
et al. using the previously seeded keys.

Change-Id: I9e74f1b38d1e9d587b296f4b973666e7ffb9b68f
This commit is contained in:
Dave Borowitz
2012-03-13 12:30:25 -07:00
parent 3dc69cbdf4
commit 0f13eb2fe7
1 changed files with 6 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
gerrit-server/src/main/java/com/google/gerrit/server/util/GuiceRequestScopePropagator.java
View File

@@ -36,9 +36,9 @@ import javax.annotation.Nullable;
/** Propagator for Guice's built-in servlet scope. */
public class GuiceRequestScopePropagator extends RequestScopePropagator {
private final Provider<String> urlProvider;
private final Provider<SocketAddress> remotePeerProvider;
private final Provider<CurrentUser> currentUserProvider;
private final String url;
private final SocketAddress peer;
private final CurrentUser user;
@Inject
GuiceRequestScopePropagator(
@@ -46,9 +46,9 @@ public class GuiceRequestScopePropagator extends RequestScopePropagator {
@RemotePeer Provider<SocketAddress> remotePeerProvider,
Provider<CurrentUser> currentUserProvider) {
super(ServletScopes.REQUEST);
this.urlProvider = urlProvider;
this.remotePeerProvider = remotePeerProvider;
this.currentUserProvider = currentUserProvider;
this.url = urlProvider != null ? urlProvider.get() : null;
this.peer = remotePeerProvider.get();
this.user = currentUserProvider.get();
}
/**
@@ -61,17 +61,14 @@ public class GuiceRequestScopePropagator extends RequestScopePropagator {
// Request scopes appear to use specific keys in their map, instead of only
// providers. Add bindings for both the key to the instance directly and the
// provider to the instance to be safe.
String url = urlProvider.get();
seedMap.put(Key.get(typeOfProvider(String.class), CanonicalWebUrl.class),
Providers.of(url));
seedMap.put(Key.get(String.class, CanonicalWebUrl.class), url);
SocketAddress peer = remotePeerProvider.get();
seedMap.put(Key.get(typeOfProvider(SocketAddress.class), RemotePeer.class),
Providers.of(peer));
seedMap.put(Key.get(SocketAddress.class, RemotePeer.class), peer);
CurrentUser user = currentUserProvider.get();
seedMap.put(Key.get(typeOfProvider(CurrentUser.class)), Providers.of(user));
seedMap.put(Key.get(CurrentUser.class), user);