| Summary: | [otre] overriding Team.isActive() may cause deadlock | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Tools] Objectteams | Reporter: | Stephan Herrmann <stephan.herrmann> | ||||
| Component: | OTJ | Assignee: | Stephan Herrmann <stephan.herrmann> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | Keywords: | noteworthy | ||||
| Version: | 0.7 | ||||||
| Target Milestone: | 0.7.1 | ||||||
| Hardware: | PC | ||||||
| OS: | Linux | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
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.
Patch has been committed as r767 and r768. Marking as noteworthy to signal that this is an API change needing documentation. Verified using I201009211735 by code inspection of o.o.Team and TeamMethodGenerator. |
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.