Dr J A Gow : ole32: Fix stream ref counting.
Alexandre Julliard
julliard at wine.codeweavers.com
Tue Feb 21 05:04:10 CST 2006
Module: wine
Branch: refs/heads/master
Commit: e3af1227c9debfaf2290be75201ccfc47de61270
URL: http://source.winehq.org/git/?p=wine.git;a=commit;h=e3af1227c9debfaf2290be75201ccfc47de61270
Author: Dr J A Gow <J.A.Gow at furrybubble.co.uk>
Date: Tue Feb 21 17:03:36 2006 +0900
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.
---
dlls/ole32/stg_stream.c | 74 ++++++++++++++++++++++++++++++++++++++++--
dlls/ole32/storage32.c | 53 ++++++++++++++++++++++++++++++
dlls/ole32/storage32.h | 20 +++++++++++
dlls/ole32/tests/storage32.c | 4 --
4 files changed, 144 insertions(+), 7 deletions(-)
diff --git a/dlls/ole32/stg_stream.c b/dlls/ole32/stg_stream.c
index 19b8bda..fd75ee2 100644
--- a/dlls/ole32/stg_stream.c
+++ b/dlls/ole32/stg_stream.c
@@ -58,8 +58,21 @@ static void StgStreamImpl_Destroy(StgStr
/*
* 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;
/*
@@ -447,6 +460,14 @@ static HRESULT WINAPI StgStreamImpl_Seek
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
* in the code below.
@@ -506,6 +527,10 @@ static HRESULT WINAPI StgStreamImpl_SetS
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_Copy
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_Copy
/*
* Sanity check
*/
+
+ if(!This->parentStorage) {
+ return STG_E_REVERTED;
+ }
+
if ( pstm == 0 )
return STG_E_INVALIDPOINTER;
@@ -691,6 +722,11 @@ static HRESULT WINAPI StgStreamImpl_Comm
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_Lock
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_Unlo
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;
}
@@ -747,6 +795,14 @@ static HRESULT WINAPI StgStreamImpl_Stat
BOOL readSucessful;
/*
+ * if stream has no parent, return STG_E_REVERTED
+ */
+
+ if(!This->parentStorage) {
+ return STG_E_REVERTED;
+ }
+
+ /*
* Read the information from the property.
*/
readSucessful = StorageImpl_ReadProperty(This->parentStorage->ancestorStorage,
@@ -791,6 +847,11 @@ static HRESULT WINAPI StgStreamImpl_Clon
/*
* 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;
diff --git a/dlls/ole32/storage32.c b/dlls/ole32/storage32.c
index 08084e4..ae01174 100644
--- a/dlls/ole32/storage32.c
+++ b/dlls/ole32/storage32.c
@@ -393,6 +393,12 @@ HRESULT WINAPI StorageBaseImpl_OpenStrea
*/
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_CreateStr
* 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( IStorag
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);
+ }
+}
/*********************************************************************
@@ -2256,6 +2295,12 @@ HRESULT StorageImpl_Construct(
memset(This, 0, sizeof(StorageImpl));
/*
+ * Initialize stream list
+ */
+
+ list_init(&This->base.strmHead);
+
+ /*
* Initialize the virtual function table.
*/
This->base.lpVtbl = &Storage32Impl_Vtbl;
@@ -2448,6 +2493,8 @@ void StorageImpl_Destroy(StorageBaseImpl
StorageImpl *This = (StorageImpl*) iface;
TRACE("(%p)\n", This);
+ StorageBaseImpl_DeleteAll(&This->base);
+
HeapFree(GetProcessHeap(), 0, This->pwcsName);
BlockChainStream_Destroy(This->smallBlockRootChain);
@@ -4113,6 +4160,12 @@ StorageInternalImpl* StorageInternalImpl
memset(newStorage, 0, sizeof(StorageInternalImpl));
/*
+ * Initialize the stream list
+ */
+
+ list_init(&newStorage->base.strmHead);
+
+ /*
* Initialize the virtual function table.
*/
newStorage->base.lpVtbl = &Storage32InternalImpl_Vtbl;
diff --git a/dlls/ole32/storage32.h b/dlls/ole32/storage32.h
index 0472524..f153021 100644
--- a/dlls/ole32/storage32.h
+++ b/dlls/ole32/storage32.h
@@ -37,6 +37,7 @@
#include "objbase.h"
#include "winreg.h"
#include "winternl.h"
+#include "wine/list.h"
/*
* Definitions for the file format offsets.
@@ -220,6 +221,12 @@ struct StorageBaseImpl
const IPropertySetStorageVtbl *pssVtbl; /* interface for adding a properties stream */
/*
+ * Stream tracking list
+ */
+
+ struct list strmHead;
+
+ /*
* Reference count of this object
*/
LONG ref;
@@ -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.
@@ -485,6 +499,12 @@ struct StgStreamImpl
* 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
*/
LONG ref;
diff --git a/dlls/ole32/tests/storage32.c b/dlls/ole32/tests/storage32.c
index b4edf13..235ca63 100644
--- a/dlls/ole32/tests/storage32.c
+++ b/dlls/ole32/tests/storage32.c
@@ -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);
More information about the wine-cvs
mailing list