diff --git a/CHANGELOG.md b/CHANGELOG.md index bb356fce20f..32afac6547a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ - Leave `inApp` flag for stack frames undecided in SDK if unsure and let ingestion decide instead ([#2547](https://github.com/getsentry/sentry-java/pull/2547)) - Allow `0.0` error sample rate ([#2573](https://github.com/getsentry/sentry-java/pull/2573)) +- Fix memory leak in WebFlux related to an ever growing stack ([#2580](https://github.com/getsentry/sentry-java/pull/2580)) - Use the same hub in WebFlux exception handler as we do in WebFilter ([#2566](https://github.com/getsentry/sentry-java/pull/2566)) ## 6.14.0 diff --git a/sentry-samples/sentry-samples-spring-boot-webflux/src/main/java/io/sentry/samples/spring/boot/SentryDemoApplication.java b/sentry-samples/sentry-samples-spring-boot-webflux/src/main/java/io/sentry/samples/spring/boot/SentryDemoApplication.java index 14e57e0cc30..f7828e8aaba 100644 --- a/sentry-samples/sentry-samples-spring-boot-webflux/src/main/java/io/sentry/samples/spring/boot/SentryDemoApplication.java +++ b/sentry-samples/sentry-samples-spring-boot-webflux/src/main/java/io/sentry/samples/spring/boot/SentryDemoApplication.java @@ -2,10 +2,17 @@ import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.context.annotation.Bean; +import org.springframework.web.reactive.function.client.WebClient; @SpringBootApplication public class SentryDemoApplication { public static void main(String[] args) { SpringApplication.run(SentryDemoApplication.class, args); } + + @Bean + WebClient webClient(WebClient.Builder builder) { + return builder.build(); + } } diff --git a/sentry-samples/sentry-samples-spring-boot-webflux/src/main/java/io/sentry/samples/spring/boot/Todo.java b/sentry-samples/sentry-samples-spring-boot-webflux/src/main/java/io/sentry/samples/spring/boot/Todo.java new file mode 100644 index 00000000000..e9b02ece2a1 --- /dev/null +++ b/sentry-samples/sentry-samples-spring-boot-webflux/src/main/java/io/sentry/samples/spring/boot/Todo.java @@ -0,0 +1,25 @@ +package io.sentry.samples.spring.boot; + +public class Todo { + private final Long id; + private final String title; + private final boolean completed; + + public Todo(Long id, String title, boolean completed) { + this.id = id; + this.title = title; + this.completed = completed; + } + + public Long getId() { + return id; + } + + public String getTitle() { + return title; + } + + public boolean isCompleted() { + return completed; + } +} diff --git a/sentry-samples/sentry-samples-spring-boot-webflux/src/main/java/io/sentry/samples/spring/boot/TodoController.java b/sentry-samples/sentry-samples-spring-boot-webflux/src/main/java/io/sentry/samples/spring/boot/TodoController.java new file mode 100644 index 00000000000..fed2f8dec56 --- /dev/null +++ b/sentry-samples/sentry-samples-spring-boot-webflux/src/main/java/io/sentry/samples/spring/boot/TodoController.java @@ -0,0 +1,26 @@ +package io.sentry.samples.spring.boot; + +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.reactive.function.client.WebClient; +import reactor.core.publisher.Mono; + +@RestController +public class TodoController { + private final WebClient webClient; + + public TodoController(WebClient webClient) { + this.webClient = webClient; + } + + @GetMapping("/todo-webclient/{id}") + Mono todoWebClient(@PathVariable Long id) { + return webClient + .get() + .uri("https://jsonplaceholder.typicode.com/todos/{id}", id) + .retrieve() + .bodyToMono(Todo.class) + .map(response -> response); + } +} diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java new file mode 100644 index 00000000000..23b2000d202 --- /dev/null +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java @@ -0,0 +1,52 @@ +package io.sentry.spring.jakarta.webflux; + +import com.jakewharton.nopen.annotation.Open; + +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.http.server.reactive.ServerHttpResponse; +import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.server.WebFilter; +import org.springframework.web.server.WebFilterChain; + +import io.sentry.Breadcrumb; +import io.sentry.Hint; +import io.sentry.IHub; +import io.sentry.Sentry; +import static io.sentry.TypeCheckHint.WEBFLUX_FILTER_REQUEST; +import static io.sentry.TypeCheckHint.WEBFLUX_FILTER_RESPONSE; +import io.sentry.util.Objects; +import reactor.core.publisher.Mono; + +/** Manages {@link io.sentry.Scope} in Webflux request processing. */ +@ApiStatus.Experimental +public abstract class AbstractSentryWebFilter implements WebFilter { + private final @NotNull SentryRequestResolver sentryRequestResolver; + public static final String SENTRY_HUB_KEY = "sentry-hub"; + + public AbstractSentryWebFilter(final @NotNull IHub hub) { + Objects.requireNonNull(hub, "hub is required"); + this.sentryRequestResolver = new SentryRequestResolver(hub); + } + + protected void doFinally(final @NotNull IHub requestHub) { + requestHub.popScope(); + } + + protected void doFirst(final @NotNull ServerWebExchange serverWebExchange, final @NotNull IHub requestHub) { + serverWebExchange.getAttributes().put(SENTRY_HUB_KEY, requestHub); + requestHub.pushScope(); + final ServerHttpRequest request = serverWebExchange.getRequest(); + final ServerHttpResponse response = serverWebExchange.getResponse(); + + final Hint hint = new Hint(); + hint.set(WEBFLUX_FILTER_REQUEST, request); + hint.set(WEBFLUX_FILTER_RESPONSE, response); + final String methodName = + request.getMethod() != null ? request.getMethod().name() : "unknown"; + requestHub.addBreadcrumb(Breadcrumb.http(request.getURI().toString(), methodName), hint); + requestHub.configureScope( + scope -> scope.setRequest(sentryRequestResolver.resolveSentryRequest(request))); + } +} diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilter.java index 78e9d4f6241..1a43f7182c6 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilter.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilter.java @@ -2,6 +2,7 @@ import com.jakewharton.nopen.annotation.Open; +import io.sentry.NoOpHub; import io.sentry.Sentry; import static io.sentry.TypeCheckHint.WEBFLUX_FILTER_REQUEST; import static io.sentry.TypeCheckHint.WEBFLUX_FILTER_RESPONSE; @@ -22,41 +23,28 @@ /** Manages {@link io.sentry.Scope} in Webflux request processing. */ @ApiStatus.Experimental @Open -public class SentryWebFilter implements WebFilter { - private final @NotNull IHub hub; - private final @NotNull SentryRequestResolver sentryRequestResolver; - public static final String SENTRY_HUB_KEY = "sentry-hub"; +public class SentryWebFilter extends AbstractSentryWebFilter { public SentryWebFilter(final @NotNull IHub hub) { - this.hub = Objects.requireNonNull(hub, "hub is required"); - this.sentryRequestResolver = new SentryRequestResolver(hub); + super(hub); } @Override public Mono filter( final @NotNull ServerWebExchange serverWebExchange, final @NotNull WebFilterChain webFilterChain) { + @NotNull IHub requestHub = Sentry.cloneMainHub(); return webFilterChain .filter(serverWebExchange) .doFinally( __ -> { - hub.popScope(); + doFinally(requestHub); + Sentry.setCurrentHub(NoOpHub.getInstance()); }) .doFirst( () -> { - serverWebExchange.getAttributes().put(SENTRY_HUB_KEY, Sentry.getCurrentHub()); - hub.pushScope(); - final ServerHttpRequest request = serverWebExchange.getRequest(); - final ServerHttpResponse response = serverWebExchange.getResponse(); - - final Hint hint = new Hint(); - hint.set(WEBFLUX_FILTER_REQUEST, request); - hint.set(WEBFLUX_FILTER_RESPONSE, response); - final String methodName = - request.getMethod() != null ? request.getMethod().name() : "unknown"; - hub.addBreadcrumb(Breadcrumb.http(request.getURI().toString(), methodName), hint); - hub.configureScope( - scope -> scope.setRequest(sentryRequestResolver.resolveSentryRequest(request))); + Sentry.setCurrentHub(requestHub); + doFirst(serverWebExchange, requestHub); }); } } diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilterWithThreadLocalAccessor.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilterWithThreadLocalAccessor.java index 419378c25cd..014e3934229 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilterWithThreadLocalAccessor.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilterWithThreadLocalAccessor.java @@ -6,11 +6,13 @@ import org.springframework.web.server.WebFilterChain; import io.sentry.IHub; +import io.sentry.NoOpHub; +import io.sentry.Sentry; import reactor.core.publisher.Mono; /** Manages {@link io.sentry.Scope} in Webflux request processing. */ @ApiStatus.Experimental -public final class SentryWebFilterWithThreadLocalAccessor extends SentryWebFilter { +public final class SentryWebFilterWithThreadLocalAccessor extends AbstractSentryWebFilter { public SentryWebFilterWithThreadLocalAccessor(final @NotNull IHub hub) { super(hub); @@ -20,6 +22,16 @@ public SentryWebFilterWithThreadLocalAccessor(final @NotNull IHub hub) { public Mono filter( final @NotNull ServerWebExchange serverWebExchange, final @NotNull WebFilterChain webFilterChain) { - return ReactorUtils.withSentryNewMainHubClone(super.filter(serverWebExchange, webFilterChain)); + return ReactorUtils.withSentryNewMainHubClone(webFilterChain + .filter(serverWebExchange) + .doFinally( + __ -> { + doFinally(Sentry.getCurrentHub()); + Sentry.setCurrentHub(NoOpHub.getInstance()); + }) + .doFirst( + () -> { + doFirst(serverWebExchange, Sentry.getCurrentHub()); + })); } } diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java index f98b26efef3..640fd62816a 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java @@ -6,6 +6,8 @@ import io.sentry.Breadcrumb; import io.sentry.Hint; import io.sentry.IHub; +import io.sentry.NoOpHub; +import io.sentry.Sentry; import io.sentry.util.Objects; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -19,11 +21,11 @@ /** Manages {@link io.sentry.Scope} in Webflux request processing. */ @ApiStatus.Experimental public final class SentryWebFilter implements WebFilter { - private final @NotNull IHub hub; + private final @NotNull SentryRequestResolver sentryRequestResolver; public SentryWebFilter(final @NotNull IHub hub) { - this.hub = Objects.requireNonNull(hub, "hub is required"); + Objects.requireNonNull(hub, "hub is required"); this.sentryRequestResolver = new SentryRequestResolver(hub); } @@ -31,15 +33,18 @@ public SentryWebFilter(final @NotNull IHub hub) { public Mono filter( final @NotNull ServerWebExchange serverWebExchange, final @NotNull WebFilterChain webFilterChain) { + @NotNull IHub requestHub = Sentry.cloneMainHub(); return webFilterChain .filter(serverWebExchange) .doFinally( __ -> { - hub.popScope(); + requestHub.popScope(); + Sentry.setCurrentHub(NoOpHub.getInstance()); }) .doFirst( () -> { - hub.pushScope(); + Sentry.setCurrentHub(requestHub); + requestHub.pushScope(); final ServerHttpRequest request = serverWebExchange.getRequest(); final ServerHttpResponse response = serverWebExchange.getResponse(); @@ -48,8 +53,9 @@ public Mono filter( hint.set(WEBFLUX_FILTER_RESPONSE, response); final String methodName = request.getMethod() != null ? request.getMethod().name() : "unknown"; - hub.addBreadcrumb(Breadcrumb.http(request.getURI().toString(), methodName), hint); - hub.configureScope( + requestHub.addBreadcrumb( + Breadcrumb.http(request.getURI().toString(), methodName), hint); + requestHub.configureScope( scope -> scope.setRequest(sentryRequestResolver.resolveSentryRequest(request))); }); }