#653 OCF server keeps or loses the wrong IStorage around save/load (HandsOffStorage/SaveCompleted not forwarded to OCF)

[Ognyan Chernokozhev]

Report

Erwin Lotter reported this in [bugs:#51]:

For TOleWindow derived classes which override EvOcViewSavePart() or EvOcViewLoadPart() it is suggested in the Borland tutorial to include

cpp if (!ocLoad.Remember) { OcDoc->SetStorage((IStorage*)0); }

after the save or load is done. In fact, one has to call OcDoc->SetStorage((IStorage*)0); unconditionally. Otherwise, the container cannot save the compound document.

Calling SetStorage(0) unconditionally hides the symptom. It does not fix the cause: OCF never tells a server when the container wants it to release or switch its storage.

Analysis (trunk)

1. On load, Remember is always true. TOcRemView::Init passes TOcSaveLoad(initInfo->Storage, true, true) (ocremvie.cpp), so the tutorial's if (!ocLoad.Remember) branch never runs. The server's TOcDocument keeps a reference to the container's sub-storage for the object's lifetime. The OLE protocol allows that, provided the server releases the storage when asked.

2. The server is never asked to release or switch storage.

3. BOleSite::HandsOffStorage (BOCOLE/bolesite.cpp) releases only BOCOLE's own pStg and sets fHandsOff. Nothing reaches OCF, so TOcDocument::Storage keeps its reference.
4. BOleSite::SaveCompleted(pStgNew) ignores pStgNew unless fHandsOff is set. A container that calls SaveCompleted(newStg) after a Save As does not make the object switch storages, neither in BOCOLE nor in OCF.

5. The only signal is Remember (= fHandsOff) in the next EvOcViewSavePart, and it arrives only if the container called HandsOffStorage first.

6. OCF's own container never calls HandsOffStorage. In BOlePart::Save (BOCOLE/bolepart.cpp) the call sits inside #if 0 ("not used in outline example or mfc source"). Between an OCF container and an OCF server, Save As therefore reaches the server as a "save copy" (SameAsLoad == false, Remember == false). The server goes on referencing the sub-storage of the old file.

7. The tutorial's save pattern is wrong. OcDoc->SetStorage(ocSave.StorageI) uses the default remember = true, which discards the document's current storage even for a save copy. Two examples:

8. Clipboard or drag data: BOleSite::GetDataHere → OleSave(..., FALSE) into a temporary STGM_DELETEONRELEASE docfile.
9. Save As, see (3).

Afterwards the document either holds the temporary or foreign storage, or, after SetStorage(0), none at all. TOcDocument::SaveParts returns true and writes nothing when Storage is null. TOleView::EvOcViewSavePart (oleview.cpp) does this correctly with SetStorage(storage, SameAsLoad || Remember) and RestoreStorage(). TOleWindow's default handlers do nothing, so every TOleWindow server has to write this code itself and is likely to copy the tutorial.

These gaps fit the reported symptom: the server pins a storage the container expects to have back, and the container's next save of that file fails. The exact failing call has not been reproduced yet.

Why the unconditional SetStorage(0) is not the fix

• It drops a storage that the server may keep under the OLE protocol, and that a server which is also a container needs in order to load its own embedded parts lazily (TOcPart::FinishLoading).
• Until the next save or load, the server has no storage, and SaveParts silently does nothing.
• It does not help TOleView-based servers, which have the same gaps (1) to (3).

Suggested work

1. Reproduce: use an OCF container and a TOleWindow-based server built with the tutorial pattern (both EXE and DLL server). Embed an object, then Save, Save As, Save again, copy the object to the clipboard and Save. Record which call fails and with which HRESULT. Repeat with the TOleView pattern, and with a non-OCF container (e.g. Word or WordPad), which does call HandsOffStorage.
2. Forward HandsOff and SaveCompleted to OCF: for example, an IBPart notification or a new OC_VIEW... event, raised from BOleSite::HandsOffStorage and from SaveCompleted(pStgNew). Default handlers in TOleWindow and TOleView would release OcDoc's storage on hands-off and adopt the new storage on SaveCompleted.
3. Make BOleSite::SaveCompleted honour a non-null pStgNew in the normal (no-scribble) state, as IPersistStorage requires.
4. Reconsider the #if 0 in BOlePart::Save: for Save As, the OLE sequence is OleSave(newStg, FALSE), then HandsOffStorage(), then SaveCompleted(newStg).
5. Documentation: correct the tutorial and document the TOleView pattern for TOleWindow servers. Possibly also give TOleWindow default storage handling, so servers only override the data save and load.

[Ognyan Chernokozhev]

Reproduced and fixed in trunk [r9013].

Reproduction. I used a headless harness with a real OCF server object, created through OCF's own class manager the way BOCOLE's class factory creates it. The harness then plays the container and drives IPersistStorage on real compound files. The failure in [bugs:#51] is this sequence:

The container loads or saves the object.
The container calls HandsOffStorage().
The container renames the object's sub-storage, as containers do when they reorganize the file on save.
Step 3 fails with STG_E_ACCESSDENIED because the server still has that storage open. This happens with both the TTT/tutorial pattern and TOleView. With the reporter's unconditional SetStorage(0) it works.

Causes

BOleSite::HandsOffStorage released only BOCOLE's own pointer. OCF was never told, so TOcDocument, and for doc/view also TStorageDocument, kept the storage open.
BOleSite::SaveCompleted(pStgNew) ignored pStgNew unless hands-off mode was set. OCF's own container does Save As as Save(new, FALSE) followed by SaveCompleted(new), without HandsOffStorage (the #if 0 in BOlePart::Save). Both BOCOLE and the server therefore kept the old, already reverted storage.
After a save copy, such as the clipboard rendering in BOleSite::GetDataHere, a server using the TTT/tutorial pattern kept the temporary copy storage. That happens because SetStorage(stg) defaults to remember = true.
Fix

BOCOLE forwards HandsOffStorage and SaveCompleted to the part through a new optional interface, IBPartStorage (IID 0x02A12A in the BOle GUID range). It is found with QueryInterface, so older OCF builds keep working.
BOleSite now honours SaveCompleted(new) outside hands-off mode.
On SaveCompleted(NULL), BOleSite tells the part which storage it keeps.
If the part does not support the interface (older OCF), BOleSite saves into the new storage with remember set, as it already did after hands-off.
TOcRemView implements the interface and raises a new server event, OC_VIEWSETSTORAGE (EvOcViewSetStorage(IStorage*), where 0 means release).
If the host does not handle the event, TOcRemView switches or releases its TOcDocument's storage. TOleWindow-based servers need no code for this.
TOleView handles the event to update its TOleDocument.
Servers no longer need SetStorage(0) in EvOcViewLoadPart/EvOcViewSavePart. Code that already calls it keeps working.

Testing. The harness checks Save, Save As in both the MFC/Word sequence and the OCF sequence, after InitNew and after Load, HandsOff followed by a rename, and save copy. It covers five server patterns: tutorial, unconditional SetStorage(0), TOleView-like, no storage handling, and a real TOleDocument/TOleView. There are 258 checks. The results are the same for x64 Unicode and x86 ANSI:

OCF BOCOLE Failures
old old 18
old new 10 (the fallback fixes Save As; HandsOff cannot reach old OCF)
new old 18 (no regressions)
new new 0
Note: BOCOLE has to be rebuilt to get the fix. A new OCF with an old BOCOLE behaves as before.

Not covered: a server that is also a container keeps its nested parts' sub-storages open after hands-off.