Bug 324537 - [otre] overriding Team.isActive() may cause deadlock
Summary: [otre] overriding Team.isActive() may cause deadlock
Status: VERIFIED FIXED
Alias: None
Product: Objectteams
Classification: Tools
Component: OTJ (show other bugs)
Version: 0.7   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 0.7.1   Edit
Assignee: Stephan Herrmann CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy
Depends on:
Blocks:
 
Reported: 2010-09-05 12:30 EDT by Stephan Herrmann CLA
Modified: 2010-09-23 16:48 EDT (History)
0 users

See Also:


Attachments
proposed API change (3.91 KB, patch)
2010-09-05 13:11 EDT, Stephan Herrmann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Herrmann CLA 2010-09-05 12:30:13 EDT
I just observed the following deadlock in test5317_concurrentActivation1():
"Thread-1":
        at T5317ca1_1._OT$removeTeam(T5317ca1_1.java)
        - waiting to lock <0xb0ed77b8> (a java.lang.Class for T5317ca1_1)
        at Team5317ca1_1._OT$unregisterFromBases(Team5317ca1_1.java)
        at org.objectteams.Team.doUnregistration(Team.java:384)
        at org.objectteams.Team._OT$implicitlyDeactivate(Team.java:282)
        - locked <0xa61c6928> (a java.lang.Object)
        at Team5317ca1_1.call(Team5317ca1_1.java:18)
        at T5317ca1Main1$Run5317ca1_1.run(T5317ca1Main1.java:9)
        at java.lang.Thread.run(Thread.java:619)
"Thread-0":
        at org.objectteams.Team._OT$implicitlyActivate(Team.java:233)
        - waiting to lock <0xa61c6928> (a java.lang.Object)
        at Team5317ca1_1.isActive(Team5317ca1_1.java)
        at org.objectteams.Team.isActive(Team.java:309)
        at T5317ca1_2.test2b(T5317ca1_2.java)
        - locked <0xb0ed77b8> (a java.lang.Class for T5317ca1_1)
        at T5317ca1_2.test2a(T5317ca1_2.java:4)
        at Team5317ca1_1$__OT__R2.test2(Team5317ca1_1.java:11)
        at Team5317ca1_1$__OT__R1.test1(Team5317ca1_1.java:7)
        at Team5317ca1_1.call(Team5317ca1_1.java:18)
        at T5317ca1Main1$Run5317ca1_1.run(T5317ca1Main1.java:9)
        at java.lang.Thread.run(Thread.java:619)

This happens in a version of this test where Team5317ca1_1 defines the
following override:
  public boolean isActive(Thread t) {
    try { Thread.sleep(100); } 
    catch (InterruptedException ie) {}
    return super.isActive(t);
  }

Since this method is invoked from a synchronized block of the initial
callin wrapper, it may not be a good idea to override it with time
comsuming work.
Comment 1 Stephan Herrmann CLA 2010-09-05 13:11:47 EDT
Created attachment 178230 [details]
proposed API change

This patch makes all isActivate methods final (empty args and with 
Thread arg -- source version and generated from TeamMethodGenerator).

In some contrived scenarios overriding *might* be cool, but the risk
of deadlocks between user code and loadtime-generated code (initial
callin wrapper) definitely weighs heavier.
Comment 2 Stephan Herrmann CLA 2010-09-05 13:15:11 EDT
Patch has been committed as r767 and r768.
Marking as noteworthy to signal that this is an API change needing
documentation.
Comment 3 Stephan Herrmann CLA 2010-09-23 16:48:34 EDT
Verified using I201009211735

by code inspection of o.o.Team and TeamMethodGenerator.