Skip to main content

[JIRA] Issue Comment Edited: (JSR_333-74) Potential deadlock in ObservationManager.removeEventListener()

  • From: "rhauch (JIRA)" <jira-no-reply@...>
  • To: issues@...
  • Subject: [JIRA] Issue Comment Edited: (JSR_333-74) Potential deadlock in ObservationManager.removeEventListener()
  • Date: Mon, 16 Dec 2013 15:42:49 +0000 (UTC)
  • Auto-submitted: auto-generated


    [ 
https://java.net/jira/browse/JSR_333-74?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=371652#action_371652
 ] 

rhauch edited comment on JSR_333-74 at 12/16/13 3:41 PM:
---------------------------------------------------------

I agree that blocking doesn't really make much sense, especially because the 
phrase "until the listener has completed executing" is not clearly defined. 
What does it mean for a listener that is continually receiving additional 
lists of events (via the event iterator) to "complete executing".

But the proposed solutions still seem overly complicated.

Isn't the intention to simply require that a listener's 
"onEvent(EventIterator)" method should be atomic -- once called with an 
iterator, the set of events as seen by the listener shouldn't be changed by 
deregistration. This includes the case when the listener deregisters itself 
from within an "onEvent" method.

My personal opinion is that calling "removeListener" should simply require 
that the specific listener's "onEvent" method will no longer be called once 
the listener is successfully deregistered. That definition is simple and is 
independent of who calls the method and what the listener might actually be 
doing at the time the method is called.

My proposal would be to change the behavior to no longer block, and to change 
the JavaDoc as follows:

{quote}
Deregisters an event listener such that its <code>onEvent</code> no longer
be called. 
<p>
This method has no effect if the listener is not registered at the time
this method is called.
</p>

@param listener The listener to deregister.
@throws RepositoryException If an error occurs.
{quote}



      was (Author: rhauch):
    I agree that blocking doesn't really make much sense, especially because 
the phrase "until the listener has completed executing" is not clearly 
defined. What does it mean for a listener that is continually receiving 
additional lists of events (via the event iterator) to "complete executing".

But the proposed solutions still seem overly complicated.

Isn't the intention to simply require that a listener's 
"onEvent(EventIterator)" method should be atomic -- once called with an 
iterator, the set of events as seen by the listener shouldn't be changed by 
deregistration. This includes the case when the listener deregisters itself 
from within an "onEvent" method.

My personal opinion is that calling "removeListener" should simply require 
that the specific listener's "onEvent" method will no longer be called once 
the listener is successfully deregistered. That definition is simple and is 
independent of who calls the method and what the listener might actually be 
doing at the time the method is called.

My specific proposal would be to change the behavior to no longer block, and 
to change the JavaDoc as follows:

{quote}
Deregisters an event listener such that its <code>onEvent</code> no longer
be called. 
<p>
This method has no effect if the listener is not registered at the time
this method is called.
</p>

@param listener The listener to deregister.
@throws RepositoryException If an error occurs.
{quote}


  
> Potential deadlock in ObservationManager.removeEventListener()
> --------------------------------------------------------------
>
>                 Key: JSR_333-74
>                 URL: https://java.net/jira/browse/JSR_333-74
>             Project: jsr-333
>          Issue Type: Bug
>          Components: api
>    Affects Versions: current
>            Reporter: jukka.zitting
>
> As discussed in https://issues.apache.org/jira/browse/OAK-1290, the ;
> ObservationManager.removeEventListener() method can lead to a deadlock 
> because it "will block until the listener has completed executing." The 
> deadlock occurs if the thread that calls this method holds a lock that the 
> event listener is trying to acquire. The relevant javadoc paragraph is:
> bq. A listener may be deregistered while it is being executed. The 
> deregistration method will block until the listener has completed 
> executing. An exception to this rule is a listener which deregisters itself 
> from within the onEvent method. In this case, the deregistration method 
> returns immediately, but deregistration will effectively be delayed until 
> the listener completes.
> I see two ways to avoid this deadlock scenario:
> # Keep the behavior as-is, but add documentation that informs clients about 
> this problem: "... To avoid potential deadlocks, the caller of this method 
> should not hold any locks that the event listener might try to acquire."
> # Change the contract so that the method should not block indefinitely: "A 
> listener may be deregistered while it is being executed. No more events 
> will be delivered to the listener once this method has returned. An 
> implementation should not block until the listener has completed executing. 
> An exception to this rule is a listener which deregisters itself from 
> within the onEvent method. In this case, the deregistration method returns 
> immediately, but deregistration will effectively be delayed until the 
> listener completes. To avoid potential deadlocks with implementations that 
> might block, the caller of this method should not hold any locks that the 
> event listener might try to acquire."
> The first is fully backwards compatible, the second safer. I'm not sure if 
> there are any clients out there that rely on the specified blocking 
> behavior.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: 
https://java.net/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        


[JIRA] Created: (JSR_333-74) Potential deadlock in ObservationManager.removeEventListener()

jukka.zitting (JIRA) 12/16/2013

[JIRA] Commented: (JSR_333-74) Potential deadlock in ObservationManager.removeEventListener()

rhauch (JIRA) 12/16/2013

[JIRA] Issue Comment Edited: (JSR_333-74) Potential deadlock in ObservationManager.removeEventListener()

rhauch (JIRA) 12/16/2013

[JIRA] Commented: (JSR_333-74) Potential deadlock in ObservationManager.removeEventListener()

jukka.zitting (JIRA) 12/16/2013
 
 
Close
loading
Please Confirm
Close