Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 349804 - Session is not invalidated after commit
Summary: Session is not invalidated after commit
Status: CLOSED FIXED
Alias: None
Product: EMF
Classification: Modeling
Component: cdo.core (show other bugs)
Version: 4.1   Edit
Hardware: Macintosh Mac OS X
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Egidijus Vaisnora CLA
QA Contact: Eike Stepper CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 350649
  Show dependency tree
 
Reported: 2011-06-20 07:09 EDT by Egidijus Vaisnora CLA
Modified: 2012-09-21 07:16 EDT (History)
3 users (show)

See Also:
stepper: review+


Attachments
Test case (5.57 KB, patch)
2011-06-20 07:21 EDT, Egidijus Vaisnora CLA
no flags Details | Diff
TestCase + patch v2 (6.28 KB, patch)
2011-06-22 09:16 EDT, Egidijus Vaisnora CLA
no flags Details | Diff
Patch v3 (6.38 KB, patch)
2011-07-02 07:09 EDT, Eike Stepper CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Egidijus Vaisnora CLA 2011-06-20 07:09:59 EDT
If failure on previous commit was encountered, then next commit doesn't invalidate the session, from which commit was executed.
Comment 1 Egidijus Vaisnora CLA 2011-06-20 07:21:28 EDT
Created attachment 198248 [details]
Test case

Test case attached. First test illustrates that invalidation on session was not executed.
Second case shows how bad it is with locking elements after such invalidation failure. Client becomes hanged (until next commit to server)
Comment 2 Egidijus Vaisnora CLA 2011-06-22 09:16:51 EDT
Created attachment 198398 [details]
TestCase + patch v2
Comment 3 Egidijus Vaisnora CLA 2011-06-22 09:28:42 EDT
Client side transaction invalidates and synchronizes commit updates by the previous commit time received from the server. Thus we need to ensure to notify commits, which has *successful* previous commit time. Patch removes lastIssuedTimeStamp and uses time stamp of the last finished transaction.
Comment 4 Eike Stepper CLA 2011-07-02 06:41:07 EDT
Changing to 4.1 to ensure that the fix will "last". Please clone this bugzilla
to 4.0 if you want a maintenance fix, too.
Comment 5 Eike Stepper CLA 2011-07-02 07:09:41 EDT
Created attachment 198999 [details]
Patch v3

Your fix looks good. But the 2. test would block the Hudson build forever in case of regressions. Please make it fail after DEFAULT_TIMEOUT_EXPECTED.

Then request a review again, please...
Comment 6 Egidijus Vaisnora CLA 2011-07-02 07:45:02 EDT
Cannot timeout particular test in API level, because lock implementation by itself uses waitForUpdate without timeout.
Comment 7 Eike Stepper CLA 2011-07-02 07:51:35 EDT
(In reply to comment #6)
> Cannot timeout particular test in API level, because lock implementation by
> itself uses waitForUpdate without timeout.

I'll approve this one when yo've filed a separate bug for the above issue in the lock implementation ;-)
Comment 8 Egidijus Vaisnora CLA 2011-07-02 07:58:57 EDT
Added bugzilla https://bugs.eclipse.org/bugs/show_bug.cgi?id=350985
Comment 9 Egidijus Vaisnora CLA 2011-07-04 02:30:52 EDT
Committed to trunk 8579 (fix), 8580 (junit test)
Comment 10 Eike Stepper CLA 2011-07-04 03:58:19 EDT
I assume you've only accidentally asked for a review again.
Comment 11 Egidijus Vaisnora CLA 2011-07-04 04:04:56 EDT
Hmm... I haven't asked for review (at least on purpose). It is already committed :)
Comment 12 Eike Stepper CLA 2012-09-21 07:16:57 EDT
Closing.