Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WFLY-11866 Exception can not be passed-by-reference #12167

Merged
merged 2 commits into from Apr 4, 2019

Conversation

@wfink
Copy link
Contributor

wfink commented Mar 19, 2019

https://issues.jboss.org/browse/WFLY-11866 is a regression of WFLY-2949.

Exceptions are not passed-by-reference if enabled, this cause issues if the Exception is not marked as Serializable.

Thanks for submitting your Pull Request!

Please make sure your PR meets the following requirements:

  • Pull Request title is properly formatted: [WFLY-XYZ] Subject or WFLY-XYZ Subject
  • Pull Request contains link to the JIRA issue(s)
  • Pull Request contains description of the issue(s)
  • Pull Request does not include fixes for issues other than the main ticket
  • Attached commits represent units of work and are properly formatted

For bigger changes, major and minor component upgrades make sure your PR also meets following requirements:

  • Pull Request requires a change to the documentation
  • Documentation have been updated accordingly
  • Tests were added to cover changes

For new features ensure as well:

  • Analysis was done
  • Test Plan has been done
  • Tests were verified in advance

If you are not an active contributor of the WildFly project you can request sponsorship by one of the members to help guide you through the process.

https://issues.jboss.org/browse/WFLY-11866 is a regression of WFLY-2949.

Exceptions are not passed-by-reference if enabled, this cause issues if the Exception is not marked as Serializable.
@bstansberry

This comment has been minimized.

Copy link
Contributor

bstansberry commented Mar 21, 2019

@chengfang or @tadamski Please review.

@tadamski

This comment has been minimized.

Copy link
Contributor

tadamski commented Mar 21, 2019

@bstansberry Apparently there was some problem with exception cloning (commit 2a5561b) so IMO we should find out what it was before merging it. @stuartwdouglas Do you remember why the change above was needed?

@jaikiran

This comment has been minimized.

Copy link
Contributor

jaikiran commented Mar 25, 2019

@bstansberry Apparently there was some problem with exception cloning (commit 2a5561b)

@tadamski I'm just guessing here - looking at Stuart's commit there, the previous issue seems to be that the exception cloning was being passed a "target type" of the invoked method's return type instead of the Exception class and I think that's what Stuart fixed.

Having looked at @wfink's commit here, to me, the changes seem fine. However, given how often this part of code is runinng into issue and also looking at the linked JIRA, how (relatively) easy it's to reproduce this, I think adding a testcase for this issue would help prevent regressions.

@tmiyargi

This comment has been minimized.

Copy link
Contributor

tmiyargi commented Mar 28, 2019

@jaikiran test in #12191

@bstansberry

This comment has been minimized.

Copy link
Contributor

bstansberry commented Apr 4, 2019

@tadamski @wfink @tmiyargi

Both the commits here were approved so once CI is done if results look good I'll merge it.

@tadamski

This comment has been minimized.

Copy link
Contributor

tadamski commented Apr 4, 2019

@bstansberry thanks!

@bstansberry bstansberry merged commit a82eabc into wildfly:master Apr 4, 2019
4 of 6 checks passed
4 of 6 checks passed
Windows - JDK 11 Finished TeamCity Build WildFly / Pull Request / Windows - JDK 11 : Execution timeout (new); tests failed: 1 (1 new), passed: 3972, ignore…
Details
Windows - JDK 8 Finished TeamCity Build WildFly / Pull Request / Windows - JDK 8 : Execution timeout; tests failed: 1 (1 new), passed: 4409, ignored: 130
Details
Linux - JDK 11 (Pull Request) - merge TeamCity build finished
Details
Linux - JDK 8 Finished TeamCity Build WildFly / Pull Request / Linux - JDK 8 : Tests passed: 4847, ignored: 134
Details
Linux - elytron - JDK 8 (Pull Request) - merge TeamCity build finished
Details
Linux with security manager - JDK 8 Finished TeamCity Build WildFly / Pull Request / Linux SM - JDK 8 : Tests passed: 4503, ignored: 152
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.