Skip to content

Navigation Menu

Sign in
Sign up

Clarify BloomFilter.put return value under concurrency - #8597

Open
NgoQuocViet2001 wants to merge 1 commit into
google:master from
NgoQuocViet2001:docs-bloomfilter-concurrent-put-contract
Open

Clarify BloomFilter.put return value under concurrency #8597
NgoQuocViet2001 wants to merge 1 commit into
google:master from
NgoQuocViet2001:docs-bloomfilter-concurrent-put-contract

Conversation

@NgoQuocViet2001

@NgoQuocViet2001 NgoQuocViet2001 commented Aug 8, 2026

Copy link
Copy Markdown

Fixes #3055.

BloomFilter.put is lock-free, so concurrent calls for the same object can each set a different subset of bits and both return true. The existing Javadoc instead states that a true result definitely marks the first insertion.

This change scopes the first-insertion and mightContain return-value guarantees to nonconcurrent calls and documents the concurrent case. Runtime behavior is unchanged.

Tests:

  • Ran: .\mvnw.cmd -B -ntp clean install (865,730 tests; 0 failures or errors)

Concurrent puts of the same object can each change different bits and return true. Limit the first-insertion and opposite-return guarantees to nonconcurrent calls and document the concurrent case.

google-cla Bot commented Aug 8, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

NgoQuocViet2001 reacted with hooray emoji

NgoQuocViet2001 force-pushed the docs-bloomfilter-concurrent-put-contract branch from f2226ae to e59b2ce Compare August 9, 2026 03:21

@MuhammadNiazAli MuhammadNiazAli left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yeh bhi ek achha open PR hai (google/guava #8597) chaliye is ka review likhte hain:

Review:

This is a meaningful correctness clarification for concurrent usage of BloomFilter.put(), and the change is well-scoped to just the Javadoc.

Core addition is the right one: the new sentence "In the absence of concurrent calls to put, ... With concurrent calls, multiple calls may return true for the same object" directly addresses a subtle race that the old doc glossed over. Without this qualifier, a reader could reasonably (and incorrectly) assume the "opposite of mightContain" guarantee always holds — which breaks down under concurrent puts racing on the same bits.
Why this matters in practice: someone using the return value of put() to decide "am I the first caller to add this object" (a common dedup pattern with Bloom filters) needs to know that under concurrency this signal isn't reliable multiple threads can all see true. Calling that out explicitly, rather than leaving it implied, is exactly the kind of thing that prevents a subtle production bug.
Wording is precise: "In the absence of concurrent calls to put" as the qualifier for the deterministic behavior, followed by the concurrent caveat at the end, reads clearly and doesn't require re-reading the whole doc block to understand which part applies when.
One thing worth double-checking: the doc doesn't say whether all concurrent callers can get true, or just "more than one" — i.e., is it possible for zero callers to get true even though the object was in fact newly added? If that's a real possibility (depends on the implementation's synchronization), it might be worth clarifying so readers don't assume "at least one caller gets true" as a hard guarantee. If the implementation guarantees at least one true, that's worth stating too, since it's the more useful invariant for callers.
Scope is clean single doc comment, no logic touched, matches the PR title exactly.

Good, precise fix. Approving, with the note above as an optional follow-up for extra clarity.

Copy link
Copy Markdown
Author

I signed it!

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

Reviewers

1 more reviewer
@MuhammadNiazAli MuhammadNiazAli MuhammadNiazAli approved these changes
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

BloomFilter multithread problem: put() NOT atomic

AltStyle によって変換されたページ (->オリジナル) /