BSTR, _bstr_t issues occuring on our Windows 2003 Server

[bluter]

We have server components which were created by a third party and
compiled in VC++5 (sp3). They run fine on NT4 and 2000, however during
testing of our migration to Server 2003, these components have been
failing when performing Data Access functions. In particular we have
been having problems with BSTR and _bstr_t.

_bstr_t(BSTR*, 1) is the original code that the problem started with.
Other findings are -

1) Converting a BSTR* to a _bstr_t when the length of the message in
BSTR* is 16,382 characters or more, gives errors.
a) Using _bstr_t(BSTR*, 1) causes the application to crash. fcopy=1
b) Using _bstr_t(BSTR*) ). In this case, fcopy=0 by default.
i) When message length is 16,382 or more characters - converts the
message but the first two characters are corrupted and unreadable
ii) When message length is much higher, ex:32,758 characters, the
application crashes.
2) The length of message content is not an issue when converting
_bstr_t to a BSTR.

_bstr_t(BSTR*, 1) worked fine even for message lengths as high as
50,292 characters, on the Windows NT 4.0 and Windows 2000 servers.

Anyone have any advice, input?

[Bronek Kozicki]

bluter wrote:
> _bstr_t(BSTR*, 1) is the original code that the problem started with.

are you sure you are using BSTR* (that is pointer to BSTR)? This is
wrong, _bstr_t is abstraction for BSTR which in itself is a pointer
(without an extra pointer). Futhermore, if you happen to mix wchar_t*
with BSTR, you may NOT pass wchar_t* where BSTR is expected, because
BSTR is expected to be prefixed with length (and wchar_t* is just a
pointer, with no magic, that is no prefix). And if you happen to pass
BSTR as a result of function, be very, very careful not to leak it or
double free; or simply replace it with _bstr_t to keep things tidy.

B.

[sunita.b...@gmail.com]

We are using BSTR and it is being passed back as a result of a
function. This is all over the project and the way it is setup, it is
close to impossible for changing it.
We do suspect that there is a leak occurring during at the time the
result is being passed back from the lower function to the calling
function. This happens in Windows 2003 server and was not an issue in
Windows NT 4.0 or Windows 2000 servers. Are you aware of similar issues
with VC++5.0 applications running on Windows 2003 servers?
Could you suggest ways to prevent leaks/double free when BSTR is passed
back as result of a function.

Code shown below:

const string FMLMsgProxy::getMsg ()
{
BSTR bstrMsg = m_pFINMsg->getMsg();
_bstr_t bstrtMsg(bstrMsg); //Code breaks here
------------------
---------------
}

STDMETHODIMP CFINMsg::getMsg(BSTR * o_Msg)
{
--------------------------------
-----------------------------
*o_Msg = m_OGMsg.copy(); //m_OGMsg is declared as _bstr_t
return S_OK;
}

[Bronek Kozicki]

sunita.b...@gmail.com wrote:
> We do suspect that there is a leak occurring during at the time the

no, but you might have a leak somewhere else, as you apparently do not
understand some things about _bstr_t and BSTR. And you have other
problems, described below.

> const string FMLMsgProxy::getMsg ()
> {
> BSTR bstrMsg = m_pFINMsg->getMsg();
> _bstr_t bstrtMsg(bstrMsg); //Code breaks here

I assume that getMsg (as called above) is actually wrapper provided by
compiler COM support, that is #import and accompanying *.tlh and *.tli
files. The wrapper funtion returns temporary object of type _bstr_t .
Just after initialization of BSTR bstrMsg this temporary object is
destroyed, and memory pointed by bstrMsg is freed by _bstr_t destructor.
Thus in next line bstrMsg is just a dangling pointer. This is why you
have crash.

Instead you should just use:

_bstr_t bstrtMsg = m_pFINMsg->getMsg();

If, on the other hand, getMsg() is not a wrapper and is *really*
returning a BSTR (not _bstr_t ), then you have a memory leak (but then
program wouldn't crash). Initialization of _bstr_t from BSTR will make a
copy and NOT take ownership of the string, unless you pass false as
second argument of constructor (that is fCopy, as documented in MSDN) -
in which case _bstr_t will take ownership of returned BSTR. If you just
use implicit conversion from BSTR to _bstr_t, you actually make a copy
of BSTR returned by COM function (instead of taking ownership), probably
no-one will free this BSTR and you have a leak.

> STDMETHODIMP CFINMsg::getMsg(BSTR * o_Msg)
> {
> --------------------------------
> -----------------------------
> *o_Msg = m_OGMsg.copy(); //m_OGMsg is declared as _bstr_t
> return S_OK;

If your function is returning a string (that is "[OUT, RETVAL] BSTRT *"
), you should pass ownership of _bstr_t . For this operation use
Detach(), not copy(). It's not that copy() or copy(true) are bad (they
aren't), but if you happen to call by mistake copy(false) or just
perform implicit converion from _bstr_t to BSTR, you will again end up
with dangling pointer. On the other hand, copy(true) is safe, but is
also wasting CPU cycles, as it allocates new string unnecessarily.
Obviously, caller of your function is resposible for freeing BSTR
returned as [OUT, RETVAL], but this is mandated by COM and provided by
COM wrappers (that is #import , *.tlh and *.tli files) so you do not
have to worry - if you use these COM wrappers properly.

I suggest that you buy this book
http://www.amazon.com/gp/product/0735611270/ (and I mean this one, not
any other - it's out of print, but there are still copies available) and
read first 3 chapters very, very carefully. At least twice. You would
benefit from other good books on COM, too.

B.

PS. some samples:

#include <comdef.h>

// fragile, caller might leak returned BSTR
BSTR foo() {_bstr_t t = L"foo"; return t.Detach();}

// fragile and unnecessary overhead making a copy
BSTR bar() {_bstr_t t = L"bar"; return t.copy();}

// very bad, returning dangling pointer
BSTR boo() {_bstr_t t = L"boo"; return t.copy(false);}

// very bad, just like boo, as operator BSTR is equiv. to copy(false)
BSTR bam() {_bstr_t t = L"bam"; return t;}

// reliable, like code provided by COM wrappers
_bstr_t bow() {_bstr_t t = L"bow"; return t;}

int main()
{
_bstr_t t(foo(), false); // OK, but fragile
_bstr_t t1 = bar(); // memory leak!
_bstr_t t2 = boo(); // crash!
_bstr_t t3 = bam(); // crash!
_bstr_t t4 = bow(); // OK and reliable
BSTR t5 = bow(); // potential trap ...
_bstr_t t6 = t5; // ... and crash!
}

obviusly, actual crash might or might not happen, depending on compiler
version, compilation options, sorrounding code etc - using dangling
pointer is always bad, however it may not always result in immediate
crash - often you will just see "bad" results, and sometimes you will
not even notice that you have a problem. This especially applies to
small sample app. that actually does nothing, just like the one above.

[sunita.b...@gmail.com]

B. wrote:
> Instead you should just use:
>
> _bstr_t bstrtMsg = m_pFINMsg->getMsg();

The code was changed to capture the BSTR into a _bstr_t wrapper and
application(s) works as expected - from what was seen in our initial
tests. The code extract that was changed is below -

const string FMLMsgProxy::getMsg ()
{

//BSTR bstrMsg = m_pFINMsg->getMsg(); // the code before
_bstr_t bstrtMsg = m_pFINMsg->getMsg(); // code after change
-------------------
--------------
}

Thanks a million ton for the informational post.

Sunita

[Bronek Kozicki]

sunita.b...@gmail.com wrote:
> //BSTR bstrMsg = m_pFINMsg->getMsg(); // the code before
> _bstr_t bstrtMsg = m_pFINMsg->getMsg(); // code after change

good.

> Thanks a million ton for the informational post.

you are welcome. Pardon my conceited tone, but I suggest that you review
whole application seeking for similar mistakes. Regards

B.