[#JETTY-1149] transient should be volatile in AbstractLifeCycle - jira.codehaus.org

Details

Type: Bug
Status: Resolved
Priority: Minor
Resolution: Fixed
Affects Version/s: 6.1.21
Fix Version/s: 6.1.23
Component/s: None
Labels:
None

Number of attachments :
0

Description

In the following line:

private transient int _state = STOPPED;

in this file:

jetty-6.1.22/modules/util/src/main/java/org/mortbay/component/AbstractLifeCycle.java

the 'transient' should very likely be 'volatile'.

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Kristian Eide added a comment - 12/Jan/10 4:21 PM

I do not consider this a minor issue since it can actually prevent Jetty from starting up. Consider the following chain of events:

server.start() is called to start Jetty.

This in turn causes an instance of Acceptor to be run in a separate thread.

Inside Acceptor.run() the method isRunning() is called, which returns false. This is because the '_state' field is not volatile and thus the new thread still sees it as 'STOPPED' even though the other thread sees it as 'STARTING. The Acceptor thread thus immediately exists and closes its socket which obviously is very undesirable.

These kinds of concurrency bugs are starting to show up on modern multi-core machines and JVMs.

Show

Kristian Eide added a comment - 12/Jan/10 4:21 PM I do not consider this a minor issue since it can actually prevent Jetty from starting up. Consider the following chain of events:

server.start() is called to start Jetty.

This in turn causes an instance of Acceptor to be run in a separate thread.

Inside Acceptor.run() the method isRunning() is called, which returns false. This is because the '_state' field is not volatile and thus the new thread still sees it as 'STOPPED' even though the other thread sees it as 'STARTING. The Acceptor thread thus immediately exists and closes its socket which obviously is very undesirable.

These kinds of concurrency bugs are starting to show up on modern multi-core machines and JVMs.

Hide

Permalink

Jan Bartel added a comment - 13/Jan/10 7:03 PM

Hi Kristian,

It seems that this was already fixed for jetty-7 and jetty-8, but not jetty-6. It's fixed now.

thanks,
Jan

Show

Jan Bartel added a comment - 13/Jan/10 7:03 PM Hi Kristian, It seems that this was already fixed for jetty-7 and jetty-8, but not jetty-6. It's fixed now. thanks, Jan

People

Assignee:

Unassigned

Reporter:

Kristian Eide

Vote (0)

Watch (0)

Dates

Created:

19/Nov/09 4:27 PM

Updated:

13/Jan/10 7:03 PM

Resolved:

13/Jan/10 7:03 PM