You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@felix.apache.org by "Sahoo (JIRA)" <ji...@apache.org> on 2010/12/27 17:30:47 UTC
[jira] Created: (FELIX-2748) Possible synchronization issue with
BundleContext.getBundle()
Possible synchronization issue with BundleContext.getBundle()
-------------------------------------------------------------
Key: FELIX-2748
URL: https://issues.apache.org/jira/browse/FELIX-2748
Project: Felix
Issue Type: Bug
Components: Framework
Affects Versions: framework-3.0.6
Reporter: Sahoo
On 12/26/10 23:24, Sahoo wrote:
> Hi,
>
> Looking at the code, it seems to me that BundleContext.getBundle() can return a bundle even though the BundleContext has been invalidated. I say this based on the following reasoning:
>
> public BundleContext.getBundle()
> {
> checkValidity();
>
> return m_bundle;
> }
>
> private void checkValidity() // non-synchronized method
> {
> if (m_valid) // m_valid is a non-volatile field
> {
> switch (m_bundle.getState())
> {
> case Bundle.ACTIVE:
> case Bundle.STARTING:
> case Bundle.STOPPING:
> return;
> }
> }
>
> throw new IllegalStateException("Invalid BundleContext.");
>
> }
>
> protected void invalidate() { m_valid = false; }
>
> class Bundle {
> public int getState() // non-synchronized method
> {
> return m_state;
> }
> }
>
> time t1: thread1 has access to bundleContext.
> time t2: thread2 calls b.update()
> time t3: bundle has been stopped and is now in STARTING state. thread1 calls bundleContext.getBundle().
> Since m_valid is not a volatile field, it can see m_valid as true, although thread2 would have set it to false. Since m_state is volatile, it sees its value as STARTING. So, it fails to detect invalidation of the bundle and is returned with the bundle object instead of IllegalStateException.
>
> Am I missing anything here?
No, I think you are correct.
I'll have to think about it a little more, but I think the real issue here is the fact that the validity of the bundle context has to be more directly tied to the bundle. They can't be treated as independent values, because even if they were both volatile or synchronized, you could still end up with a change in state between checking each value.
I think the simpler BundleContext.checkValidity() method should be something like:
m_bundle._getBundleContext() == this
Which says as long as "this" bundle context is the bundle's bundle context, then it is valid. I think. I'll still need to think about it. :-)
Want to open a bug on this issue?
Is this a theoretical exercise or are you suffering from this?
-> richard
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (FELIX-2748) Possible synchronization issue with
BundleContext.getBundle()
Posted by "Richard S. Hall (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FELIX-2748?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12975549#action_12975549 ]
Richard S. Hall commented on FELIX-2748:
----------------------------------------
Even if we modify the checkValidity() method like I suggest, I think there will still be a race condition between the check and the action (e.g., registering a listener). We need to check and make sure that each existing action either does a double check while holding the bundle lock or make it acquire the bundle lock to perform the associated action.
> Possible synchronization issue with BundleContext.getBundle()
> -------------------------------------------------------------
>
> Key: FELIX-2748
> URL: https://issues.apache.org/jira/browse/FELIX-2748
> Project: Felix
> Issue Type: Bug
> Components: Framework
> Affects Versions: framework-3.0.7
> Reporter: Sahoo
> Fix For: framework-3.2.0
>
>
> On 12/26/10 23:24, Sahoo wrote:
> > Hi,
> >
> > Looking at the code, it seems to me that BundleContext.getBundle() can return a bundle even though the BundleContext has been invalidated. I say this based on the following reasoning:
> >
> > public BundleContext.getBundle()
> > {
> > checkValidity();
> >
> > return m_bundle;
> > }
> >
> > private void checkValidity() // non-synchronized method
> > {
> > if (m_valid) // m_valid is a non-volatile field
> > {
> > switch (m_bundle.getState())
> > {
> > case Bundle.ACTIVE:
> > case Bundle.STARTING:
> > case Bundle.STOPPING:
> > return;
> > }
> > }
> >
> > throw new IllegalStateException("Invalid BundleContext.");
> >
> > }
> >
> > protected void invalidate() { m_valid = false; }
> >
> > class Bundle {
> > public int getState() // non-synchronized method
> > {
> > return m_state;
> > }
> > }
> >
> > time t1: thread1 has access to bundleContext.
> > time t2: thread2 calls b.update()
> > time t3: bundle has been stopped and is now in STARTING state. thread1 calls bundleContext.getBundle().
> > Since m_valid is not a volatile field, it can see m_valid as true, although thread2 would have set it to false. Since m_state is volatile, it sees its value as STARTING. So, it fails to detect invalidation of the bundle and is returned with the bundle object instead of IllegalStateException.
> >
> > Am I missing anything here?
> No, I think you are correct.
> I'll have to think about it a little more, but I think the real issue here is the fact that the validity of the bundle context has to be more directly tied to the bundle. They can't be treated as independent values, because even if they were both volatile or synchronized, you could still end up with a change in state between checking each value.
> I think the simpler BundleContext.checkValidity() method should be something like:
> m_bundle._getBundleContext() == this
> Which says as long as "this" bundle context is the bundle's bundle context, then it is valid. I think. I'll still need to think about it. :-)
> Want to open a bug on this issue?
> Is this a theoretical exercise or are you suffering from this?
> -> richard
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (FELIX-2748) Possible synchronization issue with
BundleContext.getBundle()
Posted by "Richard S. Hall (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FELIX-2748?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Richard S. Hall updated FELIX-2748:
-----------------------------------
Fix Version/s: (was: framework-4.0.0)
framework-3.2.0
> Possible synchronization issue with BundleContext.getBundle()
> -------------------------------------------------------------
>
> Key: FELIX-2748
> URL: https://issues.apache.org/jira/browse/FELIX-2748
> Project: Felix
> Issue Type: Bug
> Components: Framework
> Affects Versions: framework-3.0.7
> Reporter: Sahoo
> Fix For: framework-3.2.0
>
>
> On 12/26/10 23:24, Sahoo wrote:
> > Hi,
> >
> > Looking at the code, it seems to me that BundleContext.getBundle() can return a bundle even though the BundleContext has been invalidated. I say this based on the following reasoning:
> >
> > public BundleContext.getBundle()
> > {
> > checkValidity();
> >
> > return m_bundle;
> > }
> >
> > private void checkValidity() // non-synchronized method
> > {
> > if (m_valid) // m_valid is a non-volatile field
> > {
> > switch (m_bundle.getState())
> > {
> > case Bundle.ACTIVE:
> > case Bundle.STARTING:
> > case Bundle.STOPPING:
> > return;
> > }
> > }
> >
> > throw new IllegalStateException("Invalid BundleContext.");
> >
> > }
> >
> > protected void invalidate() { m_valid = false; }
> >
> > class Bundle {
> > public int getState() // non-synchronized method
> > {
> > return m_state;
> > }
> > }
> >
> > time t1: thread1 has access to bundleContext.
> > time t2: thread2 calls b.update()
> > time t3: bundle has been stopped and is now in STARTING state. thread1 calls bundleContext.getBundle().
> > Since m_valid is not a volatile field, it can see m_valid as true, although thread2 would have set it to false. Since m_state is volatile, it sees its value as STARTING. So, it fails to detect invalidation of the bundle and is returned with the bundle object instead of IllegalStateException.
> >
> > Am I missing anything here?
> No, I think you are correct.
> I'll have to think about it a little more, but I think the real issue here is the fact that the validity of the bundle context has to be more directly tied to the bundle. They can't be treated as independent values, because even if they were both volatile or synchronized, you could still end up with a change in state between checking each value.
> I think the simpler BundleContext.checkValidity() method should be something like:
> m_bundle._getBundleContext() == this
> Which says as long as "this" bundle context is the bundle's bundle context, then it is valid. I think. I'll still need to think about it. :-)
> Want to open a bug on this issue?
> Is this a theoretical exercise or are you suffering from this?
> -> richard
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Resolved: (FELIX-2748) Possible synchronization issue with
BundleContext.getBundle()
Posted by "Richard S. Hall (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FELIX-2748?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Richard S. Hall resolved FELIX-2748.
------------------------------------
Resolution: Fixed
Assignee: Richard S. Hall
I have committed a patch for this. In some cases, I think it is ok for us to ignore the race condition since the result is no different than if the calling thread had won the race. On the other hand, for the listener-related methods, I modified the framework code to grab the bundle state lock which acts as a double check, since not doing so could leave lingering listeners from an stopped bundle. For registering services, we already grabbed the bundle state lock. Please close if you are satisfied. Thanks for reporting.
> Possible synchronization issue with BundleContext.getBundle()
> -------------------------------------------------------------
>
> Key: FELIX-2748
> URL: https://issues.apache.org/jira/browse/FELIX-2748
> Project: Felix
> Issue Type: Bug
> Components: Framework
> Affects Versions: framework-3.0.7
> Reporter: Sahoo
> Assignee: Richard S. Hall
> Fix For: framework-3.2.0
>
>
> On 12/26/10 23:24, Sahoo wrote:
> > Hi,
> >
> > Looking at the code, it seems to me that BundleContext.getBundle() can return a bundle even though the BundleContext has been invalidated. I say this based on the following reasoning:
> >
> > public BundleContext.getBundle()
> > {
> > checkValidity();
> >
> > return m_bundle;
> > }
> >
> > private void checkValidity() // non-synchronized method
> > {
> > if (m_valid) // m_valid is a non-volatile field
> > {
> > switch (m_bundle.getState())
> > {
> > case Bundle.ACTIVE:
> > case Bundle.STARTING:
> > case Bundle.STOPPING:
> > return;
> > }
> > }
> >
> > throw new IllegalStateException("Invalid BundleContext.");
> >
> > }
> >
> > protected void invalidate() { m_valid = false; }
> >
> > class Bundle {
> > public int getState() // non-synchronized method
> > {
> > return m_state;
> > }
> > }
> >
> > time t1: thread1 has access to bundleContext.
> > time t2: thread2 calls b.update()
> > time t3: bundle has been stopped and is now in STARTING state. thread1 calls bundleContext.getBundle().
> > Since m_valid is not a volatile field, it can see m_valid as true, although thread2 would have set it to false. Since m_state is volatile, it sees its value as STARTING. So, it fails to detect invalidation of the bundle and is returned with the bundle object instead of IllegalStateException.
> >
> > Am I missing anything here?
> No, I think you are correct.
> I'll have to think about it a little more, but I think the real issue here is the fact that the validity of the bundle context has to be more directly tied to the bundle. They can't be treated as independent values, because even if they were both volatile or synchronized, you could still end up with a change in state between checking each value.
> I think the simpler BundleContext.checkValidity() method should be something like:
> m_bundle._getBundleContext() == this
> Which says as long as "this" bundle context is the bundle's bundle context, then it is valid. I think. I'll still need to think about it. :-)
> Want to open a bug on this issue?
> Is this a theoretical exercise or are you suffering from this?
> -> richard
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Updated: (FELIX-2748) Possible synchronization issue with
BundleContext.getBundle()
Posted by "Richard S. Hall (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/FELIX-2748?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Richard S. Hall updated FELIX-2748:
-----------------------------------
Affects Version/s: (was: framework-3.0.6)
framework-3.0.7
Fix Version/s: framework-3.2.0
> Possible synchronization issue with BundleContext.getBundle()
> -------------------------------------------------------------
>
> Key: FELIX-2748
> URL: https://issues.apache.org/jira/browse/FELIX-2748
> Project: Felix
> Issue Type: Bug
> Components: Framework
> Affects Versions: framework-3.0.7
> Reporter: Sahoo
> Fix For: framework-3.2.0
>
>
> On 12/26/10 23:24, Sahoo wrote:
> > Hi,
> >
> > Looking at the code, it seems to me that BundleContext.getBundle() can return a bundle even though the BundleContext has been invalidated. I say this based on the following reasoning:
> >
> > public BundleContext.getBundle()
> > {
> > checkValidity();
> >
> > return m_bundle;
> > }
> >
> > private void checkValidity() // non-synchronized method
> > {
> > if (m_valid) // m_valid is a non-volatile field
> > {
> > switch (m_bundle.getState())
> > {
> > case Bundle.ACTIVE:
> > case Bundle.STARTING:
> > case Bundle.STOPPING:
> > return;
> > }
> > }
> >
> > throw new IllegalStateException("Invalid BundleContext.");
> >
> > }
> >
> > protected void invalidate() { m_valid = false; }
> >
> > class Bundle {
> > public int getState() // non-synchronized method
> > {
> > return m_state;
> > }
> > }
> >
> > time t1: thread1 has access to bundleContext.
> > time t2: thread2 calls b.update()
> > time t3: bundle has been stopped and is now in STARTING state. thread1 calls bundleContext.getBundle().
> > Since m_valid is not a volatile field, it can see m_valid as true, although thread2 would have set it to false. Since m_state is volatile, it sees its value as STARTING. So, it fails to detect invalidation of the bundle and is returned with the bundle object instead of IllegalStateException.
> >
> > Am I missing anything here?
> No, I think you are correct.
> I'll have to think about it a little more, but I think the real issue here is the fact that the validity of the bundle context has to be more directly tied to the bundle. They can't be treated as independent values, because even if they were both volatile or synchronized, you could still end up with a change in state between checking each value.
> I think the simpler BundleContext.checkValidity() method should be something like:
> m_bundle._getBundleContext() == this
> Which says as long as "this" bundle context is the bundle's bundle context, then it is valid. I think. I'll still need to think about it. :-)
> Want to open a bug on this issue?
> Is this a theoretical exercise or are you suffering from this?
> -> richard
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.