On Mon, 11 May 2026 16:36:43 GMT, Daniel Gredler <[email protected]> wrote:
>> src/java.base/share/classes/java/text/AttributedString.java line 354: >> >>> 352: * empty Map. >>> 353: */ >>> 354: public void addAttributes(Map<? extends Attribute, ?> attributes, >> >> Since we moved from `Vector` to `Map`, I think this is now susceptible to >> `ConcurrentModificationException` when called at the same time that the >> `AttributedCharacterIterator` is iterated. I don't think it matters though >> since this method isn't specified to be thread safe and >> `AttributedCharacterIterator` does not define a concurrent modification >> policy, so I think it's OK. > > I also think it's OK, but it does raise an interesting question. Per the > comments at the top of `AttributedStringIterator`, these classes makes some > effort to synchronize access to the `AttributedString`. However, there is a > gap in this protection because 2 of the 3 mutating methods (both named > `addAttribute`) synchronize by delegating to the private synchronized > `addAttributeImpl` method, while the third method (`addAttributes`) does not > go through this private method, and is missing any synchronization. I think > it might be a good idea to open a separate bug to address this > synchronization gap in `addAttributes`. Thoughts? I agree there is a synchronization gap in `addAttributes` compared with the two `addAttribute` methods. As mentioned before, the specification does not appear to make any thread-safety guarantees for `AttributedString` or `AttributedCharacterIterator`. The existing synchronization looks somewhat ad-hoc, since `addAttributes` was not synchronized and the internal comment for `AttributedCharacterIterator` simply states usage should be done with a single thread. Given that has been the case since its inception, and there have been no issues on this (to my knowledge), I think it would be OK to leave as is. Before and after this change, `addAttributes` will remain not thread safe. I understand there is something to be said about having consistency across all the add `methods`, I just think it is low priority since the driver is a very old implementation comment. Let's see what Naoto thinks as well. The other changes you made look good, thanks. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30402#discussion_r3221924569
