Bugzilla – Full Text Bug Listing
| Summary: | RecoveringCDOSession attempts recovery regardless of exception type | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Modeling] EMF | Reporter: | Caspar D. <caspar_d> | ||||
| Component: | cdo.core | Assignee: | Caspar D. <caspar_d> | ||||
| Status: | CLOSED FIXED | QA Contact: | Eike Stepper <stepper> | ||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | saulius.tvarijonas, stepper | ||||
| Version: | 4.0 | Flags: | stepper:
review+
|
||||
| Target Milestone: | --- | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Caspar D.
Side note: In bug 329254 I've just changed the commit behaviour on the server so that it should never never propagate any exception instances, but rather always use the regular commit protocol (response.result.rollbackMessage) Note: The recovery mechanism deals with 2 possible problems: one is deactivation of the sessionProtocol (intercepted with RecoveringCDOSessionImpl.sessionProtocolDeactivated), the other is the occurrence of TransportExceptions during execution of methods on the CDOSessionProtocol (intercepted with RecoveringExceptionHandler.handleException). (Which is more likely to occur is platform dependent: a deactivation is unlikely on Linux because the OS won't invalidate sockets even if the physical interface fails.) While the latter problem typically occurs in the application thread, the former does not. Therefore, when deactivation occurs and is being handled, it's possible for NPE's to occur in the application thread when it makes calls on the protocol, because it's possibly calling on the deactivated protocol which hasn't yet been replaced with a new one. The recovery mechanism itself won't do anything to prevent this. It's up to the user app to deal with this. To this end, it can listen to CDOSessionRecoveryEvents coming from the session. As long as no 'FINISHED' event has been received (call CDOSessionRecoveryEvent.getType()), it is unsafe to make any calls on the session. Created attachment 188290 [details]
Patch v1
It's a big one!
Strange, I can not apply your patch. But https://bugs.eclipse.org/bugs/attachment.cgi?id=188290&action=diff shows your idea. I'm not sure but would it be good to have IOException and IORuntimeException on that list, too? BTW. I feel that we should come up with a general exception handling strategy that enables us and our consumers to more easily and more specifically react to exceptional situations. Something in net4j.util could provide a useful and *semantic* categorization similar to what the SpringFramework does with its abstraction layer over SQL vendor specific error codes and SQLExceptions. Careful consideration of checked exception versus unchecked exception should also be part of that strategy. That would be something with a high long term effect and it would be hard to change every now and then. Hence it should be discussed at least by the whole team. I've filed bug 336318 for this discussion... (In reply to comment #4) > Strange, I can not apply your patch. But > https://bugs.eclipse.org/bugs/attachment.cgi?id=188290&action=diff shows your > idea. I'm not sure but would it be good to have IOException and > IORuntimeException on that list, too? I doubt it. What if the protocol does some IO that has nothing to do with the transport layer? The way I see it, if an IOException occurs in the transport layer, and bubbles up to this level without being wrapped in a TransportException, then apparently our existing exception-wrapping strategy in inconsistent, and we'd better fix that instead of adding various exception types here. (In reply to comment #5) > I doubt it. What if the protocol does some IO that has nothing to do > with the transport layer? Granted. > The way I see it, if an IOException occurs in the transport layer, and > bubbles up to this level without being wrapped in a TransportException, > then apparently our existing exception-wrapping strategy in inconsistent, > and we'd better fix that instead of adding various exception types here. I agree. Note that TransportException is a CDO concept. Doesn't that mean that Net4j can not propagate transport IOExceptions separately from protocol IOExceptions? Seems I forgot to approve the patch... Committed to trunk, rev. 7057 (In reply to comment #6) > Note that TransportException is a CDO concept. Doesn't that mean that > Net4j can not propagate transport IOExceptions separately from protocol > IOExceptions? I'm not sure I understand the question. But it does seem that it would make more sense for TransportException to be a Net4J type. Available in R20110608-1407 |