Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow registering a custom Predicate for determining non-blocking threads #3854

Merged

Conversation

trustin
Copy link
Contributor

@trustin trustin commented Jul 22, 2024

Related issue: #3833
Motivation:

It is currently not possible to create a non-blocking threads without implementing the reactor.core.scheduler.NonBlocking interface. Some third-party libraries and frameworks don't directly depend on reactor-core, yet they want to mark the threads they manage as non-blocking.

Modifications:

  • Added a new method Schedulers.registerNonBlockingThreadPredicate() so that a user can register their own Predicate that determines whether a given thread is non-blocking or not
    • Also added Schedulers.resetNonBlockingThreadPredicate() so that a user can unregister all previous Predicates
  • Fixed an incorrectly implemented test that doesn't really test anything:
    • SchedulersTest.isInNonBlockingThreadTrue()

Result:

  • A user can now mark their own Thread classes as non-blocking without depending on reactor-core or implementing the NonBlocking marker interface.

…hreads

Related issue: reactor#3833
Motivation:

It is currently not possible to create a non-blocking threads without
implementing the `reactor.core.scheduler.NonBlocking` interface. Some
third-party libraries and frameworks don't directly depend on
`reactor-core`, yet they want to mark the threads they manage as
non-blocking.

Modifications:

- Added a new method `Schedulers.registerNonBlockingThreadPredicate()`
  so that a user can register their own `Predicate` that determines
  whether a given thread is non-blocking or not
  - Also added `Schedulers.resetNonBlockingThreadPredicate()` so that a
    user can unregister all previous `Predicate`s
- Fixed an incorrectly implemented test that doesn't really test
  anything:
  - `SchedulersTest.isInNonBlockingThreadTrue()`

Result:

- Closes reactor#3833
- A user can now mark their own `Thread` classes as non-blocking without
  depending on `reactor-core` or implementing the `NonBlocking` marker
  interface.
@chemicL
Copy link
Member

chemicL commented Jul 22, 2024

Please run ./gradlew spotlessApply and push the license header updates.

@trustin
Copy link
Contributor Author

trustin commented Jul 23, 2024

@chemicL It looks like 3.6.x doesn't have Spotless. I can update the license headers of the modified files, but could you point me to the right license header?

@trustin
Copy link
Contributor Author

trustin commented Jul 23, 2024

By the way, I ran ./gradlew spotlessApply against the main branch and it gives me this error:

> Task :spotlessJava FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':spotlessJava'.
> org.eclipse.jgit.errors.CorruptObjectException: DIRC checksum mismatch

Environment:

❯ ./gradlew --version     
------------------------------------------------------------
Gradle 8.5
------------------------------------------------------------

Build time:   2023-11-29 14:08:57 UTC
Revision:     28aca86a7180baa17117e0e5ba01d8ea9feca598

Kotlin:       1.9.20
Groovy:       3.0.17
Ant:          Apache Ant(TM) version 1.10.13 compiled on January 4 2023
JVM:          21.0.3 (Eclipse Adoptium 21.0.3+9-LTS)
OS:           Linux 6.9.10-arch1-1 amd64

@chemicL
Copy link
Member

chemicL commented Jul 23, 2024

I'm not sure what's up with your local setup, but there's nothing fishy on our side I think. I will commit my updates and we can merge.

Here's what I did:

➜  reactor-core git:(3.6.x) gh pr checkout 3854
[...]
Switched to branch 'custom_nonblocking_thread_predicate_3.6'
➜  reactor-core git:(custom_nonblocking_thread_predicate_3.6) ./gradlew spotlessApply
[...]
BUILD SUCCESSFUL in 8s
7 actionable tasks: 2 executed, 5 up-to-date

And running git diff gives:

diff --git a/reactor-core/src/main/java/reactor/core/scheduler/Schedulers.java b/reactor-core/src/main/java/reactor/core/scheduler/Schedulers.java
index 02dfa31c5..7617cb180 100644
--- a/reactor-core/src/main/java/reactor/core/scheduler/Schedulers.java
+++ b/reactor-core/src/main/java/reactor/core/scheduler/Schedulers.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016-2023 VMware Inc. or its affiliates, All Rights Reserved.
+ * Copyright (c) 2016-2024 VMware Inc. or its affiliates, All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
diff --git a/reactor-core/src/test/java/reactor/core/scheduler/SchedulersTest.java b/reactor-core/src/test/java/reactor/core/scheduler/SchedulersTest.java
index 821fbdb5a..7232a0248 100644
--- a/reactor-core/src/test/java/reactor/core/scheduler/SchedulersTest.java
+++ b/reactor-core/src/test/java/reactor/core/scheduler/SchedulersTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016-2022 VMware Inc. or its affiliates, All Rights Reserved.
+ * Copyright (c) 2016-2024 VMware Inc. or its affiliates, All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.

@chemicL chemicL added the type/enhancement A general enhancement label Jul 23, 2024
@chemicL chemicL added this to the 3.6.9 milestone Jul 23, 2024
Copy link
Member

@chemicL chemicL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution! It's an honour, @trustin :)

@chemicL chemicL merged commit 941e324 into reactor:3.6.x Jul 23, 2024
7 checks passed
chemicL added a commit that referenced this pull request Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants