ole32: Fix stream ref counting.
Stream methods called after parent object has been closed correctly return STG_E_REVERTED. Stream refcounting fixed. Now can safely call IStorage destructor before IStream destructor and guarantee file will be closed.
This commit is contained in:
parent
c74e5a784d
commit
e3af1227c9
|
@ -58,8 +58,21 @@ static void StgStreamImpl_Destroy(StgStreamImpl* This)
|
|||
|
||||
/*
|
||||
* Release the reference we are holding on the parent storage.
|
||||
* IStorage_Release((IStorage*)This->parentStorage);
|
||||
*
|
||||
* No, don't do this. Some apps call IStorage_Release without
|
||||
* calling IStream_Release first. If we grab a reference the
|
||||
* file is not closed, and the app fails when it tries to
|
||||
* reopen the file (Easy-PC, for example). Just inform the
|
||||
* storage that we have closed the stream
|
||||
*/
|
||||
IStorage_Release((IStorage*)This->parentStorage);
|
||||
|
||||
if(This->parentStorage) {
|
||||
|
||||
StorageBaseImpl_RemoveStream(This->parentStorage, This);
|
||||
|
||||
}
|
||||
|
||||
This->parentStorage = 0;
|
||||
|
||||
/*
|
||||
|
@ -446,6 +459,14 @@ static HRESULT WINAPI StgStreamImpl_Seek(
|
|||
TRACE("(%p, %ld, %ld, %p)\n",
|
||||
iface, dlibMove.u.LowPart, dwOrigin, plibNewPosition);
|
||||
|
||||
/*
|
||||
* fail if the stream has no parent (as does windows)
|
||||
*/
|
||||
|
||||
if(!(This->parentStorage)) {
|
||||
return STG_E_REVERTED;
|
||||
}
|
||||
|
||||
/*
|
||||
* The caller is allowed to pass in NULL as the new position return value.
|
||||
* If it happens, we assign it to a dynamic variable to avoid special cases
|
||||
|
@ -506,6 +527,10 @@ static HRESULT WINAPI StgStreamImpl_SetSize(
|
|||
|
||||
TRACE("(%p, %ld)\n", iface, libNewSize.u.LowPart);
|
||||
|
||||
if(!This->parentStorage) {
|
||||
return STG_E_REVERTED;
|
||||
}
|
||||
|
||||
/*
|
||||
* As documented.
|
||||
*/
|
||||
|
@ -609,6 +634,7 @@ static HRESULT WINAPI StgStreamImpl_CopyTo(
|
|||
ULARGE_INTEGER* pcbRead, /* [out] */
|
||||
ULARGE_INTEGER* pcbWritten) /* [out] */
|
||||
{
|
||||
StgStreamImpl* const This=(StgStreamImpl*)iface;
|
||||
HRESULT hr = S_OK;
|
||||
BYTE tmpBuffer[128];
|
||||
ULONG bytesRead, bytesWritten, copySize;
|
||||
|
@ -621,6 +647,11 @@ static HRESULT WINAPI StgStreamImpl_CopyTo(
|
|||
/*
|
||||
* Sanity check
|
||||
*/
|
||||
|
||||
if(!This->parentStorage) {
|
||||
return STG_E_REVERTED;
|
||||
}
|
||||
|
||||
if ( pstm == 0 )
|
||||
return STG_E_INVALIDPOINTER;
|
||||
|
||||
|
@ -691,6 +722,11 @@ static HRESULT WINAPI StgStreamImpl_Commit(
|
|||
IStream* iface,
|
||||
DWORD grfCommitFlags) /* [in] */
|
||||
{
|
||||
StgStreamImpl* const This=(StgStreamImpl*)iface;
|
||||
|
||||
if(!This->parentStorage) {
|
||||
return STG_E_REVERTED;
|
||||
}
|
||||
return S_OK;
|
||||
}
|
||||
|
||||
|
@ -714,6 +750,12 @@ static HRESULT WINAPI StgStreamImpl_LockRegion(
|
|||
ULARGE_INTEGER cb, /* [in] */
|
||||
DWORD dwLockType) /* [in] */
|
||||
{
|
||||
StgStreamImpl* const This=(StgStreamImpl*)iface;
|
||||
|
||||
if(!This->parentStorage) {
|
||||
return STG_E_REVERTED;
|
||||
}
|
||||
|
||||
FIXME("not implemented!\n");
|
||||
return E_NOTIMPL;
|
||||
}
|
||||
|
@ -724,6 +766,12 @@ static HRESULT WINAPI StgStreamImpl_UnlockRegion(
|
|||
ULARGE_INTEGER cb, /* [in] */
|
||||
DWORD dwLockType) /* [in] */
|
||||
{
|
||||
StgStreamImpl* const This=(StgStreamImpl*)iface;
|
||||
|
||||
if(!This->parentStorage) {
|
||||
return STG_E_REVERTED;
|
||||
}
|
||||
|
||||
FIXME("not implemented!\n");
|
||||
return E_NOTIMPL;
|
||||
}
|
||||
|
@ -746,6 +794,14 @@ static HRESULT WINAPI StgStreamImpl_Stat(
|
|||
StgProperty curProperty;
|
||||
BOOL readSucessful;
|
||||
|
||||
/*
|
||||
* if stream has no parent, return STG_E_REVERTED
|
||||
*/
|
||||
|
||||
if(!This->parentStorage) {
|
||||
return STG_E_REVERTED;
|
||||
}
|
||||
|
||||
/*
|
||||
* Read the information from the property.
|
||||
*/
|
||||
|
@ -791,6 +847,11 @@ static HRESULT WINAPI StgStreamImpl_Clone(
|
|||
/*
|
||||
* Sanity check
|
||||
*/
|
||||
|
||||
if(!This->parentStorage) {
|
||||
return STG_E_REVERTED;
|
||||
}
|
||||
|
||||
if ( ppstm == 0 )
|
||||
return STG_E_INVALIDPOINTER;
|
||||
|
||||
|
@ -858,12 +919,19 @@ StgStreamImpl* StgStreamImpl_Construct(
|
|||
newStream->lpVtbl = &StgStreamImpl_Vtbl;
|
||||
newStream->ref = 0;
|
||||
|
||||
newStream->parentStorage = parentStorage;
|
||||
|
||||
/*
|
||||
* We want to nail-down the reference to the storage in case the
|
||||
* stream out-lives the storage in the client application.
|
||||
*
|
||||
* -- IStorage_AddRef((IStorage*)newStream->parentStorage);
|
||||
*
|
||||
* No, don't do this. Some apps call IStorage_Release without
|
||||
* calling IStream_Release first. If we grab a reference the
|
||||
* file is not closed, and the app fails when it tries to
|
||||
* reopen the file (Easy-PC, for example)
|
||||
*/
|
||||
newStream->parentStorage = parentStorage;
|
||||
IStorage_AddRef((IStorage*)newStream->parentStorage);
|
||||
|
||||
newStream->grfMode = grfMode;
|
||||
newStream->ownerProperty = ownerProperty;
|
||||
|
|
|
@ -393,6 +393,12 @@ HRESULT WINAPI StorageBaseImpl_OpenStream(
|
|||
*/
|
||||
IStream_AddRef(*ppstm);
|
||||
|
||||
/*
|
||||
* add us to the storage's list of active streams
|
||||
*/
|
||||
|
||||
StorageBaseImpl_AddStream(This,newStream);
|
||||
|
||||
res = S_OK;
|
||||
goto end;
|
||||
}
|
||||
|
@ -979,6 +985,11 @@ HRESULT WINAPI StorageBaseImpl_CreateStream(
|
|||
* the reference.
|
||||
*/
|
||||
IStream_AddRef(*ppstm);
|
||||
|
||||
/* add us to the storage's list of active streams
|
||||
*/
|
||||
StorageBaseImpl_AddStream(This,newStream);
|
||||
|
||||
}
|
||||
else
|
||||
{
|
||||
|
@ -1797,6 +1808,34 @@ HRESULT WINAPI StorageImpl_Stat( IStorage* iface,
|
|||
return result;
|
||||
}
|
||||
|
||||
/******************************************************************************
|
||||
* Internal stream list handlers
|
||||
*/
|
||||
|
||||
void StorageBaseImpl_AddStream(StorageBaseImpl * stg, StgStreamImpl * strm)
|
||||
{
|
||||
TRACE("Stream added (stg=%p strm=%p)\n", stg, strm);
|
||||
list_add_tail(&stg->strmHead,&strm->StrmListEntry);
|
||||
}
|
||||
|
||||
void StorageBaseImpl_RemoveStream(StorageBaseImpl * stg, StgStreamImpl * strm)
|
||||
{
|
||||
TRACE("Stream removed (stg=%p strm=%p)\n", stg,strm);
|
||||
list_remove(&(strm->StrmListEntry));
|
||||
}
|
||||
|
||||
void StorageBaseImpl_DeleteAll(StorageBaseImpl * stg)
|
||||
{
|
||||
struct list *cur, *cur2;
|
||||
StgStreamImpl *strm=NULL;
|
||||
|
||||
LIST_FOR_EACH_SAFE(cur, cur2, &stg->strmHead) {
|
||||
strm = LIST_ENTRY(cur,StgStreamImpl,StrmListEntry);
|
||||
TRACE("Streams deleted (stg=%p strm=%p next=%p prev=%p)\n", stg,strm,cur->next,cur->prev);
|
||||
strm->parentStorage = NULL;
|
||||
list_remove(cur);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/*********************************************************************
|
||||
|
@ -2255,6 +2294,12 @@ HRESULT StorageImpl_Construct(
|
|||
|
||||
memset(This, 0, sizeof(StorageImpl));
|
||||
|
||||
/*
|
||||
* Initialize stream list
|
||||
*/
|
||||
|
||||
list_init(&This->base.strmHead);
|
||||
|
||||
/*
|
||||
* Initialize the virtual function table.
|
||||
*/
|
||||
|
@ -2448,6 +2493,8 @@ void StorageImpl_Destroy(StorageBaseImpl* iface)
|
|||
StorageImpl *This = (StorageImpl*) iface;
|
||||
TRACE("(%p)\n", This);
|
||||
|
||||
StorageBaseImpl_DeleteAll(&This->base);
|
||||
|
||||
HeapFree(GetProcessHeap(), 0, This->pwcsName);
|
||||
|
||||
BlockChainStream_Destroy(This->smallBlockRootChain);
|
||||
|
@ -4112,6 +4159,12 @@ StorageInternalImpl* StorageInternalImpl_Construct(
|
|||
{
|
||||
memset(newStorage, 0, sizeof(StorageInternalImpl));
|
||||
|
||||
/*
|
||||
* Initialize the stream list
|
||||
*/
|
||||
|
||||
list_init(&newStorage->base.strmHead);
|
||||
|
||||
/*
|
||||
* Initialize the virtual function table.
|
||||
*/
|
||||
|
|
|
@ -37,6 +37,7 @@
|
|||
#include "objbase.h"
|
||||
#include "winreg.h"
|
||||
#include "winternl.h"
|
||||
#include "wine/list.h"
|
||||
|
||||
/*
|
||||
* Definitions for the file format offsets.
|
||||
|
@ -219,6 +220,12 @@ struct StorageBaseImpl
|
|||
|
||||
const IPropertySetStorageVtbl *pssVtbl; /* interface for adding a properties stream */
|
||||
|
||||
/*
|
||||
* Stream tracking list
|
||||
*/
|
||||
|
||||
struct list strmHead;
|
||||
|
||||
/*
|
||||
* Reference count of this object
|
||||
*/
|
||||
|
@ -246,6 +253,13 @@ struct StorageBaseImpl
|
|||
DWORD openFlags;
|
||||
};
|
||||
|
||||
/****************************************************************************
|
||||
* StorageBaseImpl stream list handlers
|
||||
*/
|
||||
|
||||
void StorageBaseImpl_AddStream(StorageBaseImpl * stg, StgStreamImpl * strm);
|
||||
void StorageBaseImpl_RemoveStream(StorageBaseImpl * stg, StgStreamImpl * strm);
|
||||
void StorageBaseImpl_DeleteAll(StorageBaseImpl * stg);
|
||||
|
||||
/****************************************************************************
|
||||
* Storage32Impl definitions.
|
||||
|
@ -484,6 +498,12 @@ struct StgStreamImpl
|
|||
const IStreamVtbl *lpVtbl; /* Needs to be the first item in the struct
|
||||
* since we want to cast this to an IStream pointer */
|
||||
|
||||
/*
|
||||
* We are an entry in the storage object's stream handler list
|
||||
*/
|
||||
|
||||
struct list StrmListEntry;
|
||||
|
||||
/*
|
||||
* Reference count
|
||||
*/
|
||||
|
|
|
@ -593,7 +593,6 @@ static void test_storage_refcount(void)
|
|||
r = IStorage_CreateStream(stg, stmname, STGM_SHARE_EXCLUSIVE | STGM_READWRITE, 0, 0, &stm );
|
||||
ok(r==S_OK, "IStorage->CreateStream failed\n");
|
||||
|
||||
todo_wine {
|
||||
r = IStorage_Release( stg );
|
||||
ok (r == 0, "storage not released\n");
|
||||
|
||||
|
@ -603,7 +602,6 @@ static void test_storage_refcount(void)
|
|||
|
||||
r = IStream_Stat( stm, &stat, STATFLAG_DEFAULT );
|
||||
ok (r == STG_E_REVERTED, "stat should fail\n");
|
||||
}
|
||||
|
||||
r = IStream_Release(stm);
|
||||
ok (r == 0, "stream not released\n");
|
||||
|
@ -617,10 +615,8 @@ static void test_storage_refcount(void)
|
|||
r = IStorage_OpenStream( stg, stmname, 0, STGM_SHARE_EXCLUSIVE|STGM_READWRITE, 0, &stm );
|
||||
ok(r == S_OK, "OpenStream should succeed\n");
|
||||
|
||||
todo_wine {
|
||||
r = IStorage_Release(stg);
|
||||
ok(r == 0, "wrong ref count\n");
|
||||
}
|
||||
}
|
||||
|
||||
DeleteFileW(filename);
|
||||
|
|
Loading…
Reference in New Issue