Bug 558501 - [Win32] Reduce usage of trycatch JNI flag
Summary: [Win32] Reduce usage of trycatch JNI flag
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 4.14   Edit
Hardware: PC Windows All
: P3 normal (vote)
Target Milestone: 4.15 M3   Edit
Assignee: Nikita Nemkin CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-12-20 05:16 EST by Paul Pazderski CLA
Modified: 2020-02-18 06:33 EST (History)
2 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Pazderski CLA 2019-12-20 05:16:42 EST 
Quote Nikita from bug 513075 comment 18:
> Also, trycatch flag has to go. There are exactly zero valid uses for it.

Unfortunately Nikita had not wrote much more details so I had to search myself.

Bug 329518 introduced trycatch on two native functions. (as a 'temporary work around'; 8 years ago...)

This had indeed influence on bug 513075 but from my research without the trycatch it will still hard crash. Only a bit more gracefully most of the time.

Anyway one of the trycatch was added on VtblCall(int fnNumber, long ppVtbl) which is used for any com object method without arguments which includes the often used IUnknown->AddRef and IUnknown->Release.

While I have no time to revisit bug 329518 I suggest to remove trycatch from this VtblCall and add a different named VtblCall with trycatch just for the (maybe still) failing Release invocation of bug 329518. From what I know so far SWT JNI code generator already support this plan.
Comment 1 Lars Vogel CLA 2019-12-20 05:34:56 EST 
Impressive Paul, how you get more and more into SWT native development. Thanks!
Comment 2 Nikita Nemkin CLA 2019-12-20 05:36:24 EST 
Sorry for not providing details, Paul.

trycatch is a decade old _temporary_ fix for a bug in some unspecified 3rd party native code. There's no reason to keep it.
Comment 3 Lars Vogel CLA 2019-12-20 05:38:17 EST 
(In reply to Nikita Nemkin from comment #2)
> Sorry for not providing details, Paul.
> 
> trycatch is a decade old _temporary_ fix for a bug in some unspecified 3rd
> party native code. There's no reason to keep it.

Hey! You are still around, great to hear from you. I was a bit concerned that something happened to you.
Comment 4 Paul Pazderski CLA 2019-12-20 06:13:04 EST 
I made this ticket now so I can close the other one where it started. I had not fully thought it out yet. One of the trycatch-ed methods and the more important to change back is the VtblCall.

While I still think it is possible to provide two variants, one with and one without trycatch, it is rather hard to use the distinct methods.
On one hand it will probably make the code a lot more ugly and on the other hand bug 329518 mentioned the failing call is a Release but not which one so I would have to guess anyway.

Additional the original bug was reported for Windows XP in context of embedded IE. Given all that I would prefer to simply remove trycatch on the VtblCall and assume the underlying problem was fixed in Windows, IE or client code meanwhile.
Comment 5 Eclipse Genie CLA 2020-01-25 05:32:57 EST 
New Gerrit change created: https://git.eclipse.org/r/156557