diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index ee8c267c77d..ef153a68d22 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -87,6 +87,7 @@ /internal-api/src/*/*/datadog/trace/api/env/ @DataDog/apm-java /internal-api/src/*/*/datadog/trace/api/flare/ @DataDog/apm-java /internal-api/src/*/*/datadog/trace/api/function/ @DataDog/apm-java +/internal-api/src/*/*/datadog/contract/ @DataDog/apm-java /internal-api/src/*/*/datadog/perfcontract/ @DataDog/apm-java /internal-api/src/*/*/datadog/trace/api/intake/ @DataDog/apm-java /internal-api/src/*/*/datadog/trace/api/logging/ @DataDog/apm-java diff --git a/internal-api/src/main/java/datadog/contract/Confined.java b/internal-api/src/main/java/datadog/contract/Confined.java new file mode 100644 index 00000000000..b5611fb991a --- /dev/null +++ b/internal-api/src/main/java/datadog/contract/Confined.java @@ -0,0 +1,63 @@ +package datadog.contract; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Declares where an instance of the annotated type remains valid. Using an instance outside its + * {@link Extent} is a bug: the type relies on that confinement for its own correctness, for example + * by completing through an unsynchronized flag or by carrying state that is only meaningful during + * one call. + * + *

Extents are ordered from narrowest to widest: {@link Extent#CALL}, then {@link Extent#FRAME}. + * Both imply the instance is never used from another thread. An unannotated type is unconfined. + * + *

This marker changes no runtime behavior. The checker contract below guides AI review; no + * static checker currently enforces it. As a {@link CorrectnessContract}, it has no exemption. + * + *

Compare {@link datadog.trace.api.function.NoEscape}, which says an instance should not + * be retained, for performance. A confined instance must not be retained. + * + *

Checker contract. Applies to production source sets. Tests and benchmarks that misuse a + * type deliberately, to verify its behavior, are outside the rule. + * + *

+ */ +@Documented +@CorrectnessContract +@Retention(RetentionPolicy.CLASS) +@Target(ElementType.TYPE) +public @interface Confined { + + /** The extent instances of the annotated type are confined to. */ + Extent value(); + + /** Where a confined instance remains valid, from narrowest to widest. */ + enum Extent { + /** Valid only for the duration of the call it was passed into. */ + CALL, + + /** + * Valid only in the frame that obtained it and the calls that frame makes, and completed on + * every path before that frame exits. + */ + FRAME + } +} diff --git a/internal-api/src/main/java/datadog/contract/CorrectnessContract.java b/internal-api/src/main/java/datadog/contract/CorrectnessContract.java new file mode 100644 index 00000000000..fe95392cdce --- /dev/null +++ b/internal-api/src/main/java/datadog/contract/CorrectnessContract.java @@ -0,0 +1,28 @@ +package datadog.contract; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Marks an annotation as a correctness contract for documentation and static analysis. A + * correctness contract states a rule that code must follow, in the RFC 2119 sense: a violation is a + * bug, not a tradeoff. Contract annotations change no runtime behavior. + * + *

Unlike {@link datadog.perfcontract.PerfContract}, a correctness contract has no suppression. A + * declaration that needs to break the rule shows that the contract is wrong for its type; change + * the annotation rather than exempting the use. A {@link datadog.perfcontract.SuppressPerfContract} + * that names a correctness contract is invalid, since every class in its {@code value} must be a + * perf contract. + * + *

Tools use this meta-annotation to discover contracts. This annotation defines no rule itself. + * + *

{@link RetentionPolicy#CLASS} lets tools discover contracts in dependency class files without + * exposing them through runtime reflection. + */ +@Documented +@Retention(RetentionPolicy.CLASS) +@Target(ElementType.ANNOTATION_TYPE) +public @interface CorrectnessContract {} diff --git a/utils/queue-utils/src/main/java/datadog/common/queue/BaseWorkQueue.java b/utils/queue-utils/src/main/java/datadog/common/queue/BaseWorkQueue.java index 7a0de8b5cfc..3cfc539c2d5 100644 --- a/utils/queue-utils/src/main/java/datadog/common/queue/BaseWorkQueue.java +++ b/utils/queue-utils/src/main/java/datadog/common/queue/BaseWorkQueue.java @@ -375,7 +375,7 @@ public void fill(T element) { @Override public void close() { - // Reservation confines fill and close to the reserving thread, so a plain flag suffices. + // Reservation confines fill and close to the reserving frame, so a plain flag suffices. if (!done) { done = true; queue.releasePlace(); diff --git a/utils/queue-utils/src/main/java/datadog/common/queue/Reservation.java b/utils/queue-utils/src/main/java/datadog/common/queue/Reservation.java index add79a425d6..e5328c1c66d 100644 --- a/utils/queue-utils/src/main/java/datadog/common/queue/Reservation.java +++ b/utils/queue-utils/src/main/java/datadog/common/queue/Reservation.java @@ -1,5 +1,9 @@ package datadog.common.queue; +import static datadog.contract.Confined.Extent.FRAME; + +import datadog.contract.Confined; + /** * A claimed place in a {@link WorkQueue}, for a caller whose work between claiming and filling * cannot be expressed as a {@link Producer}. @@ -10,9 +14,10 @@ * quietly and permanently, which is why this is an {@link AutoCloseable} meant for * try-with-resources. * - *

A reservation is confined to the thread that reserved it: only that thread may fill or close - * it. Completion is not synchronized, so filling or closing from another thread can store two - * elements against one place, or store an element whose place was already given back. + *

A reservation is confined to the frame that reserved it: fill or close it there, in a + * try-with-resources, and never from another thread. Completion is not synchronized, so filling or + * closing from another thread can store two elements against one place, or store an element whose + * place was already given back. * *

A refused claim is a reservation too, rather than a {@code null}, and one that quietly * discards whatever is filled into it. Nothing about the failed path throws, so the shortest @@ -35,6 +40,7 @@ * } * } */ +@Confined(FRAME) public interface Reservation extends AutoCloseable { /** diff --git a/utils/queue-utils/src/main/java/datadog/common/queue/RetryQueue.java b/utils/queue-utils/src/main/java/datadog/common/queue/RetryQueue.java index b10fba899dd..e5b9884efbd 100644 --- a/utils/queue-utils/src/main/java/datadog/common/queue/RetryQueue.java +++ b/utils/queue-utils/src/main/java/datadog/common/queue/RetryQueue.java @@ -1,13 +1,18 @@ package datadog.common.queue; +import static datadog.contract.Confined.Extent.CALL; + +import datadog.contract.Confined; import javax.annotation.Nonnull; /** * The capability to resubmit work after a consumer failure. * *

Only obtainable inside {@link RetryStrategy#onFailure}, never from a plain consumer, so - * re-enqueue-after-failure stays visibly distinct from ordinary admission. + * re-enqueue-after-failure stays visibly distinct from ordinary admission. Valid only during that + * call: it carries the failed item's attempt count, which a later or different use would misreport. */ +@Confined(CALL) public interface RetryQueue { /** * Resubmits the failed item. diff --git a/utils/queue-utils/src/main/java/datadog/common/queue/WorkQueue.java b/utils/queue-utils/src/main/java/datadog/common/queue/WorkQueue.java index d4c60e695eb..bd59e3365c2 100644 --- a/utils/queue-utils/src/main/java/datadog/common/queue/WorkQueue.java +++ b/utils/queue-utils/src/main/java/datadog/common/queue/WorkQueue.java @@ -144,8 +144,8 @@ int tryPutBatch( * *

The reservation holds no position in the backing. Its element joins the queue when filled, * so an open reservation does not block consumers. Use try-with-resources to release unfilled - * reservations; abandoning one permanently consumes its capacity. Keep the reservation on the - * reserving thread. + * reservations; abandoning one permanently consumes its capacity. Keep the reservation in the + * reserving frame. * *

A refused claim returns a non-null reservation that ignores fill. Check {@link * Reservation#granted()} before building the element.