118144 - [PluginCtrl] NPN_PostURL() with target == NULL terminates ActiveX thread

Status: RESOLVED INVALID

(Core Graveyard :: Embedding: ActiveX Wrapper, defect)

Description

[Bruce Pettit]

When NPN_PostURL() is called with non-null post data and target == NULL, the 
PostQuitMessage(0) call in OnStopBinding() results in termination of primary 
ActiveX thread along with termination of callback thread (and in debug mode, 
the termination of the browser thread as well).

I've had this experience before, and it seems to have something to do 
with "Apartment Model" threading, which restricts the ActiveX control to a 
single thread:  It can start another thread, but termination of such a thread 
also signals the end of the ballgame for the control.

One solution of the problem, which I tested in another situation, is to leave 
the ancillary thread running, requesting activity via messages to its window.  
However, in this case I'm not sure a second thread is necessary.  
CBindStatusCallback can handle the data transfer asynchronously, and provides, 
I believe, sufficient events to perform NPN notifications.

With a few overrides, the default ATL implementation of CBindStatusCallback can 
be coerced to do something like this.  I am including an attachment which uses 
such an override, though in a more limited situation.

Adam, if I can provide further information or otherwise help, please let me 
know.

Comment 1

[Bruce Pettit]

Attachment: Sample Override of ATL CBindStatusCall implementation

see bug description.

Comment 2

[Adam Lock]

It is possible this is to do with the way windows and threading model base
classes behave in ATL. There might be a circumstance where ATL sees the window I
create on the main thread to receive messages from the worker thread being
destroyed and assumes to post a WM_QUIT message to that thread also.

I will confirm the bug for now but I don't know when I will be able to look more
deeply into it. It may be because the nsURLDataCallback is derived from
CComMultiThreadModel and the nsPluginHostCtrl is from CComSingleThreadModel. If
you have time would you be able to experient changing these to be the same (e.g.
CComSingleThreadModel) and see if the problem is fixed that way?

Comment 3

[Bruce Pettit]

Hi Adam.  I will try your suggestion and let you know the result.  If this 
doesn't work, I will try one of the other options to see if I can fix it.  If I 
am successful, I will send you proposed changes before making any patches.  
Bruce.

Comment 4

[Bruce Pettit]

Changing threading model for nsURLDataCallback object did not fix the problem.  
I am proceeding with Plan A (keep the thread alive), as this should require the 
fewest changes to the code.

Comment 5

[Adam Lock]

I don't know if keeping the thread alive is the way to go either. At present the
plugin spawns one thread per URL requested. If you intend to use just one
thread, you'd have to rewrite nsURLDataCallback class so it could cope with
multiple simultaneous open channels and incoming data.

I suspect that the problem is because nsURLDataCallback is derived from
CComObjectRootEx<CComMultiThreadModel> and during destruction either this or an
incorrect definition of _Module or the incorrect use of _ATL_APARTMENT_THREADED
or something is causing something to foul up that causes the main thread to exit.

Can you attach a simple test case to this bug and I will see if I can pin it
down? I would be much happier to fix a faulty usage of ATL than to rewrite
without knowing what the problem was exactly.

Comment 6

[Bruce Pettit]

I'll put together a simple test case to demonstrate the problem and attach it 
tomorrow.

Comment 7

[Bruce Pettit]

Adam, great work!  I'll download and give it a whirl.

Comment 8

[Adam Lock]

I marked bug 115290 FIXED not this one :(

Comment 9

[Bruce Pettit]

Sorry.  Wishful thinking.  I will continue putting together a test case for 
this one.

Comment 10

[Bruce Pettit]

Just some additional information.  In creating a simple example to demonstrate 
the problem, I discovered that 1) data that was supposed to be posted was not 
reaching the cgi script, and 2) when that problem was corrected, everything 
worked perfectly.  However, it still didn't work on my "real" plug-in.

The data posting problem was corrected with some minor modifications to 
nsURLDataCallback::GetBindInfo.  Tracing through the plugin host control with 
my actual plug-in after this change was made, I discovered that the first 
thread created, to load the plug-in data, blocks on the call to URLOpenStream 
(which is a blocking call according to the documentation), and does not enter 
the message loop until all the data has been transfered.  In my case I use the 
plug-in data received to create a second NPN_PostURL(), and two threads are 
spawned and two URLOpenStream functions called before the first one returns-- 
this seems to result in the thread termination problem.  In my abbreviated 
example, because the amount of data is small, this doesn't happen and all is 
well.

I think that replacing the URLOpenStream() call with a call to 
CBindStatusCallback::StartAsyncDownload() (in the ATL default implementation of 
IBindStatusCallback) could correct the problem.  I am going to attempt this and 
will keep you posted.  At least it doesn't appear that any change is necessary 
to you multi-threaded design.

Comment 11

[Ed Morley [:emorley]]

The ActiveX embedding API was removed in bug 662023 and friends.

-> INVALID

[Filter bugspam on activexinvalid]