wqweto / VbAsyncSocket

Sockets with pure VB6 impl of TLS encryption
MIT License
165 stars 33 forks source link

Get rid of IUnknown, this allows us to drop stdole reference #41

Closed Doridian closed 6 months ago

Doridian commented 6 months ago

I am not sure what the consequences of this might be, but this allows me to drop the stdole dependency, which was causing me a headache trying to keep the .vbp file unchanging across machines.

It still compiles (I am using the "native" driver module), and still works fine for both the async control (with TLS) as well as the HTTP(S) request implementation.

If this is an incredibly stupid idea, do let me know as well :)

wqweto commented 6 months ago

. . . which was causing me a headache trying to keep the .vbp file unchanging across machines.

Keep in mind that any other kind of project reference will do this too because the .vbp contains local typelib path which might change when opening/editing the project on another machine.

Using Object data-type has some merrit out on its own but I'll have to test the cleanup thunk if it's working with member variables being declared as IDispatch i.e. if there is any additional QI sneaking.

Edit: Frankly I'm surprised this PR does not outright crash with AV as it is :-))

Doridian commented 6 months ago

. . . which was causing me a headache trying to keep the .vbp file unchanging across machines.

Keep in mind that any other kind of project reference will do this too because the .vbp contains local typelib path which might change when opening/editing the project on another machine.

Using Object data-type has some merrit out on its own but I'll have to test the cleanup thunk if it's working with member variables being declared as IDispatch i.e. if there is any additional QI sneaking.

Yeah, I am aware of that. I have managed to get rid of all references by now. The project (an old game) really doesn't need too many external things, except connecting to the server, which is why I am very glad I found this library (so I can get the game to support TLS)

Doridian commented 6 months ago

Edit: Frankly I'm surprised this PR does not outright crash with AV as it is :-))

Just saw your edit and yeah. This was literally me going "I wonder what happens if I just make all of these Object" and being surprised myself that it just worked perfectly :D

wqweto commented 6 months ago

Unfortunately this switch turned out to be pretty dangerous as both the thunks and shell streams do not implement IDispatch interface so late-bound calling any method/property on these references brings the IDE down with Access Violation.

Even hovering the mouse over say m_pCleanup or pOutput variable leads to Access Violation as the IDE tries to evaluate default member by calling IDispatch::Invoke for DISPID_VALUE.

You can keep these changes in your fork provided that you aware of the amount of instability this might bring but I cannot merge these in official repo without mitigating the risks and this will not be possible for shell streams.

Doridian commented 6 months ago

@wqweto Thanks for your patience and the thorough response. Closing this PR now :)