Class variables and concurrency

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Class variables and concurrency

levmatta-3
Hello all,
    today I found a very poor code in a webapp that I am helping to develop:
(Obvious buggy)
class Foo {
    public static String CONST_SQL;
    public static String CONST_SQL1 = "select * from a where b = ";
    public static String getA(String param) {
       CONST_SQL = CONST_SQL1 + param;
        ...run query and return result
    }
}
(corrected to)
class Foo {
    //speed is the priority not code beauty
    public static String CONST_SQL1 = "select * from a where b = ";
    //reentrant code
    public static String getA(String param) {
       String CONST_SQL = CONST_SQL1 + param;
        ...run query and return result
    }
}
    Since this method can be run by multiple threads, and we are using
class variables, this is a theoretical bug. But is it a actual one, is
there a pratical situation where this bug will manifest itself?
    I ask because since the variable is not synchronized, it appears to
me that it will get cached and will never be refreshed in any pratical
situation.

What are the rules for concurrent access to class variables, and what
how would the Sun JVM (version 1.4.3) behave?

Thanks,
Luís

PS: This will run on a powerfull machine (I just know it is new and runs
solaris) ontop of websphere.
PS2: I know that caching is system dependent and not especified, but I
am interest in the pratical side of things

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: Class variables and concurrency

Dawid Kurzyniec
Luís Matta wrote:

> (corrected to)
> class Foo {
>    //speed is the priority not code beauty
>    public static String CONST_SQL1 = "select * from a where b = ";
>    //reentrant code
>    public static String getA(String param) {
>       String CONST_SQL = CONST_SQL1 + param;
>        ...run query and return result
>    }
> }

Make CONST_SQL1 final (and perhaps private), rename CONST_SQL to
something more sensible (e.g. query), remove the comment about speed,
and you're fine. Final fields and local variables are thread-safe (do
not need synchronization).

>    Since this method can be run by multiple threads, and we are using
> class variables, this is a theoretical bug.

There is no such thing as theoretical bug. Especially when concurrency
is involved.

>
> PS: This will run on a powerfull machine (I just know it is new and
> runs solaris) ontop of websphere.
> PS2: I know that caching is system dependent and not especified, but I
> am interest in the pratical side of things
>
It is very dangerous to take concurrency so lightly. It can cost your
company a lot of money. The most subtle concurrency bugs usually do not
appear until the application is deployed and under heavy load [I'm
shamelessly borrowing from JCiP here]. And that is usually the worst
possible moment to have a system failure, resulting in your clients or
your boss wanting to hurt you and your family. It is *crucial* to design
the code carefully and to code defensively to eliminate concurrency
problems at the design level. Testing is simply not good enough. Even
more so because it seems that you are unable to run tests on the target
machine. Concurrency problems can often go unnoticed on uniprocessor
developer machines, but come to light on powerful, multiprocessor
application servers.

Regards,
Dawid Kurzyniec

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: Class variables and concurrency

serge masse
Dawid,
I thought that William Pugh
(http://www.cs.umd.edu/~pugh/papers/jmm2.pdf), Jeremy
Manson (recently in this list
http://altair.cs.oswego.edu/pipermail/concurrency-interest/2005-November/002084.html)
and with Brian Goetz
(http://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html),
stated something to the effect that *final* fields
(which includes String's) were not threadsafe in Java
1.4 down and thus require explicit synchronization
constructs to be added by programmers, like all
non-final shared fields (*final* fields are threadsafe
in Java 5 without additional code).

Is this correct about *final* fields requiring
synchronization in 1.4 down?

If this is the case, then ladies and gentlemen, we are
about to have real problem with these new multi-core
chips, aren't we? Probably much bigger than the Y2K
problem because soon most of our software is going to
migrate from single cpu chips to multiprocessor chips
and possibly much new defects will occur in
multithreaded apps. And I tend to agree with David
Homes that apps that have run correctly on SMP
(symmetric multiprocessors) are less likely to show
defects when run on multi-core chips, although the
chance is probably not zero.

Are there estimates of the risks of migrating
multithreaded apps to multi-core chips from single
processor and from SMP, and to SMP from single
processor? It would be very important to see a
percentage of new problems observed in apps that have
been ported from single to SMP (which have been in
widespread use for a while now).

I'm getting very nervous about this. For example, I
migrated my open source projects to Java 5 back in
2004 but I may still have to painfully re-test them
for multi-core chips because the Java 5 versions were
only tested on single processor systems. I also have
to give some very bad news to my clients who are
mostly still at 1.3 and 1.4.

I predict that if multithreaded Java apps show a
significant percentage of new defects when ported to
multi-core chips and dotnet apps don't (or show a
lower percentage), then virtually the only such chips
that will sell will be those that are supported by
dotnet and Java development will become a memory,
unless this issue is addressed now.

JSRs 133 and 166 are great but not sufficient for the
current average programmer.

For example, the Java community could develop a new
language or design tool that would:
- make it very easy to develop safe multithreaded apps
(new apps);
- automatically transform and render threadsafe
existing Java apps (old apps).

Are there such projects now? Is Fortress one of these
(http://research.sun.com/sunlabsday/talks.php?id=55)?
Are there others?

serge


--- Dawid Kurzyniec <[hidden email]> wrote:

> Luís Matta wrote:
>
> > (corrected to)
> > class Foo {
> >    //speed is the priority not code beauty
> >    public static String CONST_SQL1 = "select *
> from a where b = ";
> >    //reentrant code
> >    public static String getA(String param) {
> >       String CONST_SQL = CONST_SQL1 + param;
> >        ...run query and return result
> >    }
> > }
>
> Make CONST_SQL1 final (and perhaps private), rename
> CONST_SQL to
> something more sensible (e.g. query), remove the
> comment about speed,
> and you're fine. Final fields and local variables
> are thread-safe (do
> not need synchronization).
>
> >    Since this method can be run by multiple
> threads, and we are using
> > class variables, this is a theoretical bug.
>
> There is no such thing as theoretical bug.
> Especially when concurrency
> is involved.
>
> >
> > PS: This will run on a powerfull machine (I just
> know it is new and
> > runs solaris) ontop of websphere.
> > PS2: I know that caching is system dependent and
> not especified, but I
> > am interest in the pratical side of things
> >
> It is very dangerous to take concurrency so lightly.
> It can cost your
> company a lot of money. The most subtle concurrency
> bugs usually do not
> appear until the application is deployed and under
> heavy load [I'm
> shamelessly borrowing from JCiP here]. And that is
> usually the worst
> possible moment to have a system failure, resulting
> in your clients or
> your boss wanting to hurt you and your family. It is
> *crucial* to design
> the code carefully and to code defensively to
> eliminate concurrency
> problems at the design level. Testing is simply not
> good enough. Even
> more so because it seems that you are unable to run
> tests on the target
> machine. Concurrency problems can often go unnoticed
> on uniprocessor
> developer machines, but come to light on powerful,
> multiprocessor
> application servers.
>
> Regards,
> Dawid Kurzyniec
>
> _______________________________________________
> Concurrency-interest mailing list
> [hidden email]
>
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
>

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

RE: Class variables and concurrency

David Holmes
In reply to this post by levmatta-3
Not withstanding Dawid's responses I'd just like to address a couple of
points.

Luís Matta writes:

> (Obvious buggy)
> class Foo {
>     public static String CONST_SQL;
>     public static String CONST_SQL1 = "select * from a where b = ";
>     public static String getA(String param) {
>        CONST_SQL = CONST_SQL1 + param;
>         ...run query and return result
>     }
> }
> ...
>     Since this method can be run by multiple threads, and we are using
> class variables, this is a theoretical bug. But is it a actual one, is
> there a pratical situation where this bug will manifest itself?
>     I ask because since the variable is not synchronized, it appears to
> me that it will get cached and will never be refreshed in any pratical
> situation.

It would be possible for the value written to CONST_SQL to be maintained as
a local or on the stack and so if a second thread changed CONST_SQL the
first thread need not notice it. But it is just as possible that the static
field will be reloaded when used to run the actual query - in which case it
could see someone else's value for CONST_SQL and so execute the wrong query.

There are many factors at play here including the source code compiler, and
the JIT. Bottom line is this code has a serious bug.

> What are the rules for concurrent access to class variables, and what
> how would the Sun JVM (version 1.4.3) behave?

The rules for accessing statics is no different to accessing instance
fields. The source compiler will issue as many or as few GETSTATIC/PUTSTATIC
bytecodes as is needed to implement the program semantics and required by
the specification. Source code compilers tend to make very conservative
assumptions about what might happen in method calls so it is likely that use
of a static field before and after a method call would result in two
GETSTATIC uses. The JIT will again do whatever it needs to implement the
program semantics but this time with more of a view on performance and
optimization. That may or may not require that the GETSTATIC bytecodes
remain as explicit loads from memory - it depends on what analysis the
compiler does of any methods called in between the accesses.

Bottom line: don't think about these things as you have no idea what will
occur at runtime. Write the code correctly as per the language
specification.


Cheers,
David Holmes

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest