[
https://issues.apache.org/jira/browse/CASSANDRA-14871?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16714681#comment-16714681
]
Robert Stupp edited comment on CASSANDRA-14871 at 12/10/18 3:59 PM:
--------------------------------------------------------------------
I'm fine with using {{Verify}}, but it should include the string that yielded
{{null}}. Like {{Verify.verify(type != null, "Parsing %s yielded null, which is
a bug", str)}} and move that after the assignment of {{type}}. That way we
don't need the {{cache.get(str)}} in the synchronized block, because {{type}}
is then guaranteed to be non-null.
EDIT: So instead of
{code:java}
synchronized (TypeParser.class)
{
if (!cache.containsKey(str))
{
ImmutableMap.Builder<String, AbstractType<?>> builder =
ImmutableMap.builder();
builder.putAll(cache).put(str, type);
cache = builder.build();
}
AbstractType<?> rtype = cache.get(str);
Verify.verify(rtype != null);
return rtype;
}
{code}
like this:
{code:java}
if (!isEOS(str, i) && str.charAt(i) == '(')
type = getAbstractType(name, new TypeParser(str, i));
else
type = getAbstractType(name);
Verify.verify(type != null, "Parsing %s yielded null, which is a bug",
str);
// <comments>
synchronized (TypeParser.class)
{
if (!cache.containsKey(str))
{
ImmutableMap.Builder<String, AbstractType<?>> builder =
ImmutableMap.builder();
builder.putAll(cache).put(str, type);
cache = builder.build();
}
return type;
}
{code}
was (Author: snazy):
I'm fine with using {{Verify}}, but it should include the string that yielded
{{null}}. Like {{Verify.verify(type != null, "Parsing %s yielded null, which is
a bug", type)}} and move that after the assignment of {{type}}. That way we
don't need the {{cache.get(str)}} in the synchronized block, because {{type}}
is then guaranteed to be non-null.
{code:java}
synchronized (TypeParser.class)
{
if (!cache.containsKey(str))
{
ImmutableMap.Builder<String, AbstractType<?>> builder =
ImmutableMap.builder();
builder.putAll(cache).put(str, type);
cache = builder.build();
}
AbstractType<?> rtype = cache.get(str);
Verify.verify(rtype != null);
return rtype;
}
{code}
> Severe concurrency issues in STCS,DTCS,TWCS,TMD.Topology,TypeParser
> -------------------------------------------------------------------
>
> Key: CASSANDRA-14871
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14871
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Reporter: Robert Stupp
> Assignee: Robert Stupp
> Priority: Critical
> Fix For: 4.0, 3.0.x, 3.11.x
>
>
> There are a couple of places in the code base that do not respect that
> j.u.HashMap + related classes are not thread safe and some parts rely on
> internals of the implementation of HM, which can change.
> We have observed failures like {{NullPointerException}} and
> {{ConcurrentModificationException}} as well as wrong behavior.
> Affected areas in the code base:
> * {{SizeTieredCompactionStrategy}}
> * {{DateTieredCompactionStrategy}}
> * {{TimeWindowCompactionStrategy}}
> * {{TokenMetadata.Topology}}
> * {{TypeParser}}
> * streaming / concurrent access to {{LifecycleTransaction}} (handled in
> CASSANDRA-14554)
> While the patches for the compaction strategies + {{TypeParser}} are pretty
> straight forward, the patch for {{TokenMetadata.Topology}} requires it to be
> made immutable.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
