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

Make asynchronous proceed work for non-child threads in annotation-style aspects #141

Closed
kriegaex opened this issue Mar 23, 2022 · 1 comment
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@kriegaex
Copy link
Contributor

This is work related to, but out of scope of bug #128 and PR #132. See #132 (comment) and subsequent comments for details.

The same scenario works for native-syntax aspects, but not for @AspectJ-style ones. See the currently deactivated test Bugs199Tests.testAsyncProceedNestedAroundAdviceThreadPool_gh128, which simply needs to be un-ignored in order to have a ready-to-use test case for a future implementation.

@kriegaex kriegaex added the enhancement New feature or request label Mar 23, 2022
kriegaex added a commit to kriegaex/aspectj that referenced this issue Mar 23, 2022
Out of scope for PR eclipse-aspectj#132, future work for issue eclipse-aspectj#141.

Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
kriegaex added a commit that referenced this issue Mar 23, 2022
Out of scope for PR #132, future work for issue #141.

Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
kriegaex added a commit that referenced this issue Mar 6, 2024
according to https://rules.sonarsource.com/java/tag/leak/RSPEC-5164/.
Now, there no longer is a thread-local stack of AroundClosure instances,
but rather a list of them, which can only grow but never shrink.
Instead, we now have a thread-local (integer) list index, for every
thread being initialised with pointing to the last element. I.e., every
thread can unwind by decrementing the index while proceeding,
independently of other threads.

A positive side effect is that this approach also works for long-lived
threads from thread pools, used by executor services. Hence, test
Bugs199Tests.testAsyncProceedNestedAroundAdviceThreadPool_gh128, which
was previously commented out, has been activated and passes, see #141.

I am not sure if this brings @AspectJ style, non-inlined, nested around
advice execution functionally on par with native ones, but at least for
current scenarios it seems to work.

Fixes #288, #141.

Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
kriegaex added a commit that referenced this issue Mar 6, 2024
according to https://rules.sonarsource.com/java/tag/leak/RSPEC-5164/.
Now, there no longer is a thread-local stack of AroundClosure instances,
but rather a list of them, which can only grow but never shrink.
Instead, we now have a thread-local (integer) list index, for every
thread being initialised with pointing to the last element. I.e., every
thread can unwind by decrementing the index while proceeding,
independently of other threads.

A positive side effect is that this approach also works for long-lived
threads from thread pools, used by executor services. Hence, test
Bugs199Tests.testAsyncProceedNestedAroundAdviceThreadPool_gh128, which
was previously commented out, has been activated and passes, see #141.

I am not sure if this brings @AspectJ style, non-inlined, nested around
advice execution functionally on par with native ones, but at least for
current scenarios it seems to work.

Fixes #288, #141.

Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
kriegaex added a commit that referenced this issue Mar 6, 2024
according to https://rules.sonarsource.com/java/tag/leak/RSPEC-5164/.
Now, there no longer is a thread-local stack of AroundClosure instances,
but rather a list of them, which can only grow but never shrink.
Instead, we now have a thread-local (integer) list index, for every
thread being initialised with pointing to the last element. I.e., every
thread can unwind by decrementing the index while proceeding,
independently of other threads.

A positive side effect is that this approach also works for long-lived
threads from thread pools, used by executor services. Hence, test
Bugs199Tests.testAsyncProceedNestedAroundAdviceThreadPool_gh128, which
was previously commented out, has been activated and passes, see #141.

I am not sure if this brings @AspectJ style, non-inlined, nested around
advice execution functionally on par with native ones, but at least for
current scenarios it seems to work.

Fixes #288, #141.

Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
@kriegaex kriegaex self-assigned this Mar 6, 2024
@kriegaex kriegaex added this to the 1.9.21.2 milestone Mar 6, 2024
kriegaex added a commit that referenced this issue Mar 12, 2024
according to https://rules.sonarsource.com/java/tag/leak/RSPEC-5164/.
Now, there no longer is a thread-local stack of AroundClosure instances,
but rather a list of them, which can only grow but never shrink.
Instead, we now have a thread-local (integer) list index, for every
thread being initialised with pointing to the last element. I.e., every
thread can unwind by decrementing the index while proceeding,
independently of other threads.

A positive side effect is that this approach also works for long-lived
threads from thread pools, used by executor services. Hence, test
Bugs199Tests.testAsyncProceedNestedAroundAdviceThreadPool_gh128, which
was previously commented out, has been activated and passes, see #141.

I am not sure if this brings @AspectJ style, non-inlined, nested around
advice execution functionally on par with native ones, but at least for
current scenarios it seems to work.

Fixes #288, #141.

Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
@kriegaex
Copy link
Contributor Author

Fixed by #291.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant