Allow factories to customize their generated Providers

This enables a few optimizations

- A number of implementations can avoid `InternalContext` overheads addressing #1802
  - `ConstantBindings`, one time bindings like `Initializable` and `@Singleton` bound values, and a number of multibinder bindings in particular.
- We can leverage the cache within `BindingImpl` to share provider instances.

The tricky part is about multibindings, which have complex multi-phase initialization semantics, so we need to be defensive about `makeProvider` getting called prior to being initialized.  To help manage this I
* reordered the `LookupProcessor` to run _after_ multibinders.
  - The comment justifying its current location in initialization is obsolete since multibinders were rewritten to be 'native'.
  - This ensures lookups get optimized providers!
* made the `SyntheticProviderFactory` allocate its delegate lazily.
* make `OptionalBinder` explicitly initialize their delegate bindings and also be defensive against recursive and duplicate initialization.  This is necessary to force initialization to be correct when our `getExistingBinding` query ends up delegating to another multibinder.
* be defensive about initialization failing to complete due to errors

PiperOrigin-RevId: 714766545

core/src/com/google/inject/internal/BindingImpl.java

@@ -17,6 +17,7 @@
 package com.google.inject.internal;
 
 import com.google.common.base.MoreObjects;
+import com.google.errorprone.annotations.concurrent.LazyInit;
 import com.google.inject.Binding;
 import com.google.inject.Key;
 import com.google.inject.Provider;
@@ -64,16 +65,23 @@ public Object getSource() {
     return source;
   }
 
-  private volatile Provider<T> provider;
+  // We don't care if we initialize this multiple times, so we use LazyInit instead of a double
+  // checked locking pattern.
+  @LazyInit private volatile Provider<T> provider;
 
   @Override
   public Provider<T> getProvider() {
+    // Read into a local so we only read the field once.
+    var provider = this.provider;
     if (provider == null) {
       if (injector == null) {
         throw new UnsupportedOperationException("getProvider() not supported for module bindings");
       }
-
-      provider = injector.getProvider(key);
+      // NOTE: we cannot simply call internalFactory.getProvider() since that will not correctly
+      // enforce injector rules around jit bindings.  So we go through a special method in
+      // InjectorImpl to make the provider.
+      provider = injector.getProviderForBindingImpl(key);
+      this.provider = provider;
     }
     return provider;
   }

core/src/com/google/inject/internal/BindingProcessor.java

@@ -120,7 +120,7 @@ public Boolean visit(InstanceBinding<? extends T> binding) {
             InternalFactory<? extends T> factory =
                 ref.isPresent()
                     ? new InitializableFactory<>(ref.get())
-                    : new ConstantFactory<>(instance);
+                    : ConstantFactory.create(instance, source);
             InternalFactory<? extends T> scopedFactory =
                 Scoping.scope(key, injector, factory, source, scoping);
             putBinding(

core/src/com/google/inject/internal/BoundProviderFactory.java

@@ -54,6 +54,7 @@ public void notify(Errors errors) {
   public T get(InternalContext context, Dependency<?> dependency, boolean linked)
       throws InternalProvisionException {
     try {
+      // TODO: lukes - are we passing the right dependency here?
       jakarta.inject.Provider<? extends T> provider = providerFactory.get(context, dependency, true);
       return circularGet(provider, context, dependency, provisionCallback);
     } catch (InternalProvisionException ipe) {

core/src/com/google/inject/internal/ConstantFactory.java

@@ -17,23 +17,54 @@
 package com.google.inject.internal;
 
 import com.google.common.base.MoreObjects;
+import com.google.inject.Provider;
 import com.google.inject.spi.Dependency;
 
-/** @author [email protected] (Bob Lee) */
+/**
+ * @author [email protected] (Bob Lee)
+ */
 final class ConstantFactory<T> implements InternalFactory<T> {
+  private static <T> InternalFactory<T> nullFactory(Object source) {
+    return new InternalFactory<T>() {
+      @Override
+      public T get(InternalContext context, Dependency<?> dependency, boolean linked)
+          throws InternalProvisionException {
+        if (!dependency.isNullable()) {
+          InternalProvisionException.onNullInjectedIntoNonNullableDependency(source, dependency);
+        }
+        return null;
+      }
+
+      @Override
+      public Provider<T> makeProvider(InjectorImpl injector, Dependency<?> dependency) {
+        return InternalFactory.makeProviderForNull(source, this, dependency);
+      }
+    };
+  }
 
   private final T instance;
 
-  public ConstantFactory(T instance) {
+  static <T> InternalFactory<T> create(T instance, Object source) {
+    if (instance == null) {
+      return nullFactory(source);
+    }
+    return new ConstantFactory<>(instance);
+  }
+
+  private ConstantFactory(T instance) {
     this.instance = instance;
   }
 
   @Override
-  public T get(InternalContext context, Dependency<?> dependency, boolean linked)
-      throws InternalProvisionException {
+  public T get(InternalContext context, Dependency<?> dependency, boolean linked) {
     return instance;
   }
 
+  @Override
+  public Provider<T> makeProvider(InjectorImpl injector, Dependency<?> dependency) {
+    return InternalFactory.makeProviderFor(instance, this);
+  }
+
   @Override
   public String toString() {
     return MoreObjects.toStringHelper(ConstantFactory.class).add("value", instance).toString();

core/src/com/google/inject/internal/InitializableFactory.java

@@ -17,6 +17,8 @@
 package com.google.inject.internal;
 
 import com.google.common.base.MoreObjects;
+import com.google.errorprone.annotations.concurrent.LazyInit;
+import com.google.inject.Provider;
 import com.google.inject.spi.Dependency;
 
 /**
@@ -25,6 +27,12 @@
 final class InitializableFactory<T> implements InternalFactory<T> {
 
   private final Initializable<T> initializable;
+  // Cache the values here so we can optimize the behavior of Provider instances.
+  // We do not use a lock but rather a volatile field to safely publish values.  This means we
+  // might compute the value multiple times, but we rely on internal synchronization inside the
+  // singleton scope to ensure that we only compute the value once.
+  // See https://github.com/google/guice/issues/1802 for more details.
+  @LazyInit private volatile T value;
 
   public InitializableFactory(Initializable<T> initializable) {
     this.initializable = initializable;
@@ -33,9 +41,42 @@ public InitializableFactory(Initializable<T> initializable) {
   @Override
   public T get(InternalContext context, Dependency<?> dependency, boolean linked)
       throws InternalProvisionException {
+    // NOTE: we do not need to check nullishness here because Initializables never contain nulls.
     return initializable.get(context);
   }
 
+  @Override
+  public Provider<T> makeProvider(InjectorImpl injector, Dependency<?> dependency) {
+
+    var value = this.value;
+    if (value != null) {
+      return InternalFactory.makeProviderFor(value, this);
+    }
+
+    return new Provider<T>() {
+      @Override
+      public T get() {
+        // Avoid calling `enterContext` when we can.
+        var value = InitializableFactory.this.value;
+        if (value != null) {
+          return value;
+        }
+        try (InternalContext context = injector.enterContext()) {
+          value = initializable.get(context);
+          InitializableFactory.this.value = value;
+          return value;
+        } catch (InternalProvisionException e) {
+          throw e.addSource(dependency).toProvisionException();
+        }
+      }
+
+      @Override
+      public String toString() {
+        return InitializableFactory.this.toString();
+      }
+    };
+  }
+
   @Override
   public String toString() {
     return MoreObjects.toStringHelper(InitializableFactory.class)

core/src/com/google/inject/internal/InjectorImpl.java

@@ -349,7 +349,8 @@ private <T> BindingImpl<MembersInjector<T>> createMembersInjectorBinding(
             TypeLiteral.get(((ParameterizedType) membersInjectorType).getActualTypeArguments()[0]);
     MembersInjector<T> membersInjector = membersInjectorStore.get(instanceType, errors);
 
-    InternalFactory<MembersInjector<T>> factory = new ConstantFactory<>(membersInjector);
+    InternalFactory<MembersInjector<T>> factory =
+        ConstantFactory.create(membersInjector, SourceProvider.UNKNOWN_SOURCE);
 
     return new InstanceBindingImpl<MembersInjector<T>>(
         this,
@@ -388,11 +389,17 @@ private static class SyntheticProviderBindingImpl<T> extends BindingImpl<Provide
     }
 
     static <T> InternalFactory<Provider<T>> createInternalFactory(Binding<T> providedBinding) {
-      final Provider<T> provider = providedBinding.getProvider();
+      // Defer calling `getProvider` until we need it, Bindingimple has an internal cache and by
+      // delaying we can ensure we link to an optimized provider instance.
       return new InternalFactory<Provider<T>>() {
         @Override
         public Provider<T> get(InternalContext context, Dependency<?> dependency, boolean linked) {
-          return provider;
+          return providedBinding.getProvider();
+        }
+
+        @Override
+        public Provider<Provider<T>> makeProvider(InjectorImpl injector, Dependency<?> dependency) {
+          return InternalFactory.makeProviderFor(providedBinding.getProvider(), this);
         }
       };
     }
@@ -519,7 +526,7 @@ private static class ConvertedConstantBindingImpl<T> extends BindingImpl<T>
           injector,
           key,
           originalBinding.getSource(),
-          new ConstantFactory<T>(value),
+          ConstantFactory.create(value, originalBinding.getSource()),
           Scoping.UNSCOPED);
       this.value = value;
       provider = Providers.of(value);
@@ -595,7 +602,17 @@ <T> void initializeBinding(BindingImpl<T> binding, Errors errors) throws ErrorsE
     }
   }
 
-  <T> void initializeJitBinding(BindingImpl<T> binding, Errors errors) throws ErrorsException {
+  /** For multibinding bindings that delegate to each other. */
+  <T> void initializeBindingIfDelayed(Binding<T> binding, Errors errors) throws ErrorsException {
+    if (binding instanceof InternalProviderInstanceBindingImpl
+        && ((InternalProviderInstanceBindingImpl) binding).getInitializationTiming()
+            == InternalProviderInstanceBindingImpl.InitializationTiming.DELAYED) {
+      ((DelayedInitialize) binding).initialize(this, errors);
+    }
+  }
+
+  private <T> void initializeJitBinding(BindingImpl<T> binding, Errors errors)
+      throws ErrorsException {
     // Put the partially constructed binding in the map a little early. This enables us to handle
     // circular dependencies. Example: FooImpl -> BarImpl -> FooImpl.
     // Note: We don't need to synchronize on jitBindingData.lock() during injector creation.
@@ -770,7 +787,8 @@ private <T> BindingImpl<TypeLiteral<T>> createTypeLiteralBinding(
 
     @SuppressWarnings("unchecked") // by definition, innerType == T, so this is safe
     TypeLiteral<T> value = (TypeLiteral<T>) TypeLiteral.get(innerType);
-    InternalFactory<TypeLiteral<T>> factory = new ConstantFactory<>(value);
+    InternalFactory<TypeLiteral<T>> factory =
+        ConstantFactory.create(value, SourceProvider.UNKNOWN_SOURCE);
     return new InstanceBindingImpl<TypeLiteral<T>>(
         this,
         key,
@@ -1135,39 +1153,13 @@ public <T> Provider<T> getProvider(Class<T> type) {
     return getProvider(Key.get(checkNotNull(type, "type")));
   }
 
-  <T> Provider<T> getProviderOrThrow(final Dependency<T> dependency, Errors errors)
-      throws ErrorsException {
-    Key<T> key = dependency.getKey();
-    BindingImpl<? extends T> binding = getBindingOrThrow(key, errors, JitLimitation.NO_JIT);
-    final InternalFactory<? extends T> internalFactory = binding.getInternalFactory();
-
-    return new Provider<T>() {
-      @Override
-      public T get() {
-        InternalContext currentContext = enterContext();
-        try {
-          T t = internalFactory.get(currentContext, dependency, false);
-          return t;
-        } catch (InternalProvisionException e) {
-          throw e.addSource(dependency).toProvisionException();
-        } finally {
-          currentContext.close();
-        }
-      }
-
-      @Override
-      public String toString() {
-        return internalFactory.toString();
-      }
-    };
-  }
-
   @Override
   public <T> Provider<T> getProvider(final Key<T> key) {
     checkNotNull(key, "key");
     Errors errors = new Errors(key);
     try {
-      Provider<T> result = getProviderOrThrow(Dependency.get(key), errors);
+      // Access off the BindingImpl to leverage the cached provider.
+      Provider<T> result = getBindingOrThrow(key, errors, JitLimitation.NO_JIT).getProvider();
       errors.throwIfNewErrors(0);
       return result;
     } catch (ErrorsException e) {
@@ -1177,6 +1169,30 @@ public <T> Provider<T> getProvider(final Key<T> key) {
     }
   }
 
+  // A special implementation for BindingImpl to break a recursive dependency with getProvider so
+  // that getProvider can leverage the cache inside BindingImpl
+  <T> Provider<T> getProviderForBindingImpl(Key<T> key) {
+    Errors errors = new Errors(key);
+    try {
+      return getProviderOrThrow(Dependency.get(key), errors);
+    } catch (ErrorsException e) {
+      ConfigurationException exception =
+          new ConfigurationException(errors.merge(e.getErrors()).getMessages());
+      throw exception;
+    }
+  }
+
+  // Used by LookupProcessor to satisfy delegates.
+  <T> Provider<T> getProviderOrThrow(Dependency<T> dependency, Errors errors)
+      throws ErrorsException {
+    var key = dependency.getKey();
+    BindingImpl<T> binding = getBindingOrThrow(key, errors, JitLimitation.NO_JIT);
+    @SuppressWarnings("unchecked") // safe because Providers are covariant.
+    Provider<T> provider =
+        (Provider<T>) binding.getInternalFactory().makeProvider(this, dependency);
+    return provider;
+  }
+
   @Override
   public <T> T getInstance(Key<T> key) {
     return getProvider(key).get();

core/src/com/google/inject/internal/InjectorShell.java

@@ -277,6 +277,11 @@ public Injector get() {
       return injector;
     }
 
+    @Override
+    public Provider<Injector> makeProvider(InjectorImpl injector, Dependency<?> dependency) {
+      return InternalFactory.makeProviderFor(injector, this);
+    }
+
     @Override
     public String toString() {
       return "Provider<Injector>";
@@ -307,17 +312,26 @@ private static void bindLogger(InjectorImpl injector) {
   private static class LoggerFactory implements InternalFactory<Logger>, Provider<Logger> {
     @Override
     public Logger get(InternalContext context, Dependency<?> dependency, boolean linked) {
-      InjectionPoint injectionPoint = dependency.getInjectionPoint();
-      return injectionPoint == null
-          ? Logger.getAnonymousLogger()
-          : Logger.getLogger(injectionPoint.getMember().getDeclaringClass().getName());
+      return makeLogger(dependency);
     }
 
     @Override
     public Logger get() {
       return Logger.getAnonymousLogger();
     }
 
+    @Override
+    public Provider<Logger> makeProvider(InjectorImpl injector, Dependency<?> dependency) {
+      return InternalFactory.makeProviderFor(makeLogger(dependency), this);
+    }
+
+    private static Logger makeLogger(Dependency<?> dependency) {
+      InjectionPoint injectionPoint = dependency.getInjectionPoint();
+      return injectionPoint == null
+          ? Logger.getAnonymousLogger()
+          : Logger.getLogger(injectionPoint.getMember().getDeclaringClass().getName());
+    }
+
     @Override
     public String toString() {
       return "Provider<Logger>";
@@ -331,7 +345,7 @@ private static void bindStage(InjectorImpl injector, Stage stage) {
             injector,
             key,
             SourceProvider.UNKNOWN_SOURCE,
-            new ConstantFactory<Stage>(stage),
+            ConstantFactory.create(stage, SourceProvider.UNKNOWN_SOURCE),
             ImmutableSet.<InjectionPoint>of(),
             stage);
     injector.getBindingData().putBinding(key, stageBinding);