Skip to content

Conversation

@wendigo
Copy link
Contributor

@wendigo wendigo commented Nov 28, 2025

Description

Additional context and related issues

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Nov 28, 2025
@wendigo wendigo requested a review from findepi November 28, 2025 20:55
@wendigo wendigo force-pushed the serafin/scoped-value branch from 51fcd56 to b5a6252 Compare November 29, 2025 12:21
@wendigo wendigo force-pushed the serafin/scoped-value branch from b5a6252 to 4cdc446 Compare November 29, 2025 12:28
@wendigo wendigo requested a review from losipiuk December 1, 2025 10:02
@wendigo wendigo merged commit 56e1e16 into trinodb:master Dec 2, 2025
63 checks passed
@github-actions github-actions bot added this to the 479 milestone Dec 2, 2025
@findepi
Copy link
Member

findepi commented Dec 2, 2025

ThreadLocals (TL) distinguish between inheritable (InheritableThreadLocal) and non-inheritable (the default). Only the non-inheritable TLs behave reasonably, especially when thread pools are involved. The inheritable TLs definitely have some safe application patterns, but I am yet to learn them.

ScopedValue are a much more convenient API than ThreadLocal. (ScopedValue<T> is like ThreadLocal<Stack<T>> with nice API).
However, they are not a safe replacement for TL. This is because ScopedValue are always inheritable (via StructuredTaskScope).

Here is a program demonstrating that ReentrantBoundedExecutor using ScopedValue no longer enforces its bounds.

static void main()
{
    ReentrantBoundedExecutor executor = new ReentrantBoundedExecutor(
            Executors.newCachedThreadPool(),
            1);

    executor.execute(() -> {
        // This looks rogue, but that can be some other somewhere else using StructuredTaskScope to transport some other ScopedValue to some other place
        try (var scope = StructuredTaskScope.open()) {
            CyclicBarrier barrier = new CyclicBarrier(2);
            var first = scope.fork(() -> {
                executor.execute(() -> {
                    System.out.println("currentThread().getName() = " + currentThread().getName());
                    try {
                        barrier.await();
                    }
                    catch (Exception e) {
                        throw new RuntimeException(e);
                    }
                    System.out.println("two threads are in");
                });
            });
            var second = scope.fork(() -> {
                executor.execute(() -> {
                    System.out.println("currentThread().getName() = " + currentThread().getName());
                    try {
                        barrier.await();
                    }
                    catch (Exception e) {
                        throw new RuntimeException(e);
                    }
                    System.out.println("two threads are in");
                });
            });
            scope.join();
        }
        catch (Exception e) {
            throw new RuntimeException(e);
        }
    });
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants