[picocontainer-dev] Commit 5799 Race Condition Fix....

Discussion:

Michael Rimov

2011-11-08 00:43:08 UTC

Paul,

I was looking @ the diffs.... and this code in Iterative adapter:

if (injectionMembers == null) {
synchronized (this) {
if (injectionMembers == null) {
initializeInjectionMembersAndTypeLists();
}
}
}

Isn't that double-check locking and not really solving the race condition?
Or is Boolean assignment exempt from this kind of situation?

-Mike

-----Original Message-----
From: Paul Hammant [mailto:paul-POq8DFUn+***@public.gmane.org]
Sent: Monday, November 07, 2011 11:53 AM
To: dev-qxt/k92ZUMzOYGyH7mwfjrEhcVWVsK+***@public.gmane.org
Subject: Re: [picocontainer-dev] Release of Pico 2.14

My bad - I've only just done the nexus 'approval'. Give it four hours or
so. Meanwhile build from source :

http://svn.codehaus.org/picocontainer/java/2.x/tags/picocontainer-2.14/
mvn clean install -DskipTests -Preporting

Hi Paul,
Will this release be deployed in Maven repository ?
Thanks

Hey Mike. The answer is I'm not sure. It's pretty up to date - go
for it: The stuff that's in the latest release isn't in 3.x / Git
yet.
- Paul
On Mon, Nov 7, 2011 at 10:38 AM, Michael Rimov

Hey Paul,
What is the last change you ported over to Pico 3?
I'll be happy to manually port based on the diffs, but I wanted to
make sure before I started that I wasn't duplicating efforts.

---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

http://xircles.codehaus.org/manage_email

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

http://xircles.codehaus.org/manage_email

Paul Hammant

2011-11-08 01:31:41 UTC

Permalink

I eliminated the situation I was getting in a production deployment. I'm
not sure what you mean Mike.

-ph

Post by Michael Rimov
Paul,
if (injectionMembers == null) {
synchronized (this) {
if (injectionMembers == null) {
initializeInjectionMembersAndTypeLists();
}
}
}
Isn't that double-check locking and not really solving the race condition?
Or is Boolean assignment exempt from this kind of situation?
-Mike
-----Original Message-----
Sent: Monday, November 07, 2011 11:53 AM
Subject: Re: [picocontainer-dev] Release of Pico 2.14
My bad - I've only just done the nexus 'approval'. Give it four hours or
http://svn.codehaus.org/picocontainer/java/2.x/tags/picocontainer-2.14/
mvn clean install -DskipTests -Preporting

Hi Paul,
Will this release be deployed in Maven repository ?
Thanks

Hey Mike. The answer is I'm not sure. It's pretty up to date - go
for it: The stuff that's in the latest release isn't in 3.x / Git
yet.
- Paul
On Mon, Nov 7, 2011 at 10:38 AM, Michael Rimov

Hey Paul,
What is the last change you ported over to Pico 3?
I'll be happy to manually port based on the diffs, but I wanted to
make sure before I started that I wasn't duplicating efforts.

---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email

---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email
---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email

peter royal

2011-11-08 01:42:01 UTC

Permalink

it's safe in modern vms as long as injectionMembers is declared volatile

-pete
--

I eliminated the situation I was getting in a production deployment. I'm not sure what you mean Mike.
-ph
Paul,
if (injectionMembers == null) {
synchronized (this) {
if (injectionMembers == null) {
initializeInjectionMembersAndTypeLists();
}
}
}
Isn't that double-check locking and not really solving the race condition?
Or is Boolean assignment exempt from this kind of situation?
-Mike
-----Original Message-----
Sent: Monday, November 07, 2011 11:53 AM
Subject: Re: [picocontainer-dev] Release of Pico 2.14
My bad - I've only just done the nexus 'approval'. Give it four hours or
http://svn.codehaus.org/picocontainer/java/2.x/tags/picocontainer-2.14/
mvn clean install -DskipTests -Preporting

Hi Paul,
Will this release be deployed in Maven repository ?
Thanks

Hey Mike. The answer is I'm not sure. It's pretty up to date - go
for it: The stuff that's in the latest release isn't in 3.x / Git
yet.
- Paul
On Mon, Nov 7, 2011 at 10:38 AM, Michael Rimov

Hey Paul,
What is the last change you ported over to Pico 3?
I'll be happy to manually port based on the diffs, but I wanted to
make sure before I started that I wasn't duplicating efforts.

---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email

Michael Rimov

2011-11-08 02:07:32 UTC

Permalink

Oh thatâs good, I remember when even volatile wasnât enough and you had to synchronize the whole methodâŠ. I donât miss the days of JDK 1.0 (rofl)

However, the variable âinitializedâ wasnât declared as volatile, so Iâve done it and checked it inâŠ as I patch pico 3 Iâll go over it and see if thereâs more items like that and Iâll get that fixed.

Paul: Sorry I had the name wrong, itâs âdouble-checked lockingâ, not âdouble-lock checkingâ Anyway, if you arenât familiar with it, Wikipediaâs writeup isnât too shabby:

http://en.wikipedia.org/wiki/Double-checked_locking

When Iâm done going through the code, Iâll let you guys know so you can release a 2.14.1 if we donât get other things fixed when Iâm done.

-Mike

From: peter royal [mailto:peter.royal-e+***@public.gmane.org]
Sent: Monday, November 07, 2011 5:42 PM
To: dev-qxt/k92ZUMzOYGyH7mwfjrEhcVWVsK+***@public.gmane.org
Subject: Re: [picocontainer-dev] Commit 5799 Race Condition Fix....

it's safe in modern vms as long as injectionMembers is declared volatile

-pete
--
(peter.royal|osi)@pobox.com - http://fotap.org/~osi

On Nov 7, 2011, at 7:31 PM, Paul Hammant <paul-POq8DFUn+***@public.gmane.org> wrote:

I eliminated the situation I was getting in a production deployment. I'm not sure what you mean Mike.

-ph

On Mon, Nov 7, 2011 at 6:43 PM, Michael Rimov <rimovm-***@public.gmane.org> wrote:

Paul,

I was looking @ the diffs.... and this code in Iterative adapter:

if (injectionMembers == null) {
synchronized (this) {
if (injectionMembers == null) {
initializeInjectionMembersAndTypeLists();
}
}
}

Isn't that double-check locking and not really solving the race condition?
Or is Boolean assignment exempt from this kind of situation?

-Mike

-----Original Message-----
From: Paul Hammant [mailto:paul-POq8DFUn+***@public.gmane.org]
Sent: Monday, November 07, 2011 11:53 AM
To: dev-qxt/k92ZUMzOYGyH7mwfjrEhcVWVsK+***@public.gmane.org
Subject: Re: [picocontainer-dev] Release of Pico 2.14

My bad - I've only just done the nexus 'approval'. Give it four hours or
so. Meanwhile build from source :

http://svn.codehaus.org/picocontainer/java/2.x/tags/picocontainer-2.14/
mvn clean install -DskipTests -Preporting

Hi Paul,
Will this release be deployed in Maven repository ?
Thanks

Hey Mike. The answer is I'm not sure. It's pretty up to date - go
for it: The stuff that's in the latest release isn't in 3.x / Git
yet.
- Paul
On Mon, Nov 7, 2011 at 10:38 AM, Michael Rimov

Hey Paul,
What is the last change you ported over to Pico 3?
I'll be happy to manually port based on the diffs, but I wanted to
make sure before I started that I wasn't duplicating efforts.

---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email

Paul Hammant

2011-11-08 16:58:30 UTC

Permalink

Yup that wikipedia entry highlights a bug. Good catch & thanks for
weighing in Pete.

There's another bug-fix that should perhaps make it into a 2.14.1 release -
PICO-382 and PICO-384. I've looked at the former's testcase and compiled
it for the sake of a test. I'm somewhat hogtied at the moment for
interactive debugging as JetBrains's haven't given Codehaus a new Intellij
license yet :-(

- Paul

Oh thats good, I remember when even volatile wasnt enough and you had to
synchronize the whole method. I dont miss the days of JDK 1.0 (rofl)****
** **
However, the variable initialized wasnt declared as volatile, so Ive
done it and checked it in as I patch pico 3 Ill go over it and see if
theres more items like that and Ill get that fixed.****
** **
Paul: Sorry I had the name wrong, its double-checked locking, not
double-lock checking Anyway, if you arent familiar with it, Wikipedias
writeup isnt too shabby:****
http://en.wikipedia.org/wiki/Double-checked_locking****
** **
When Im done going through the code, Ill let you guys know so you can
release a 2.14.1 if we dont get other things fixed when Im done.****
** **
-Mike****
** **
** **
*Sent:* Monday, November 07, 2011 5:42 PM
*Subject:* Re: [picocontainer-dev] Commit 5799 Race Condition Fix....****
** **
it's safe in modern vms as long as injectionMembers is declared volatile**
**
** **
-pete
-- ****
I eliminated the situation I was getting in a production deployment. I'm
not sure what you mean Mike.****
** **
-ph****
wrote:****
Paul,
if (injectionMembers == null) {
synchronized (this) {
if (injectionMembers == null) {
initializeInjectionMembersAndTypeLists();
}
}
}
Isn't that double-check locking and not really solving the race condition?
Or is Boolean assignment exempt from this kind of situation?
-Mike
-----Original Message-----
Sent: Monday, November 07, 2011 11:53 AM
Subject: Re: [picocontainer-dev] Release of Pico 2.14
My bad - I've only just done the nexus 'approval'. Give it four hours or
http://svn.codehaus.org/picocontainer/java/2.x/tags/picocontainer-2.14/
mvn clean install -DskipTests -Preporting

Hi Paul,
Will this release be deployed in Maven repository ?
Thanks

Hey Mike. The answer is I'm not sure. It's pretty up to date - go
for it: The stuff that's in the latest release isn't in 3.x / Git
yet.
- Paul
On Mon, Nov 7, 2011 at 10:38 AM, Michael Rimov

Hey Paul,
What is the last change you ported over to Pico 3?
I'll be happy to manually port based on the diffs, but I wanted to
make sure before I started that I wasn't duplicating efforts.

---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email