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

Add default protection from unsafe code in the AfterLoad method #39

Open
jeff2001 opened this issue Nov 14, 2013 · 0 comments
Open

Add default protection from unsafe code in the AfterLoad method #39

jeff2001 opened this issue Nov 14, 2013 · 0 comments

Comments

@jeff2001
Copy link
Contributor

Redmine Feature #782

Putting code in the AfterLoad method of a BO that loads other BOs can potentially be unsafe.
It will also almost always have some sort of a performance hit as well.
There have been quite a few instances of intermittent bugs that cause deadlocks or the like when developers have been unaware of the dangers.

The proposed solution here is to have a variable called something like "AllowUnsafeAfterLoad" which will be false by default.
When the afterload method is executed there would be some sort of flag that would be set to indicate that we are currently "DuringAfterLoad".
If there are any relationships loaded on the object or any other potentially unsafe actions (maybe any loader calls whatsoever...assess this...) then a developer error will be thrown unless the "AllowUnsafeAfterLoad" flag has been set to true. The error that is thrown should explain why it has been thrown and inform the developer of the dangers of unsafe afterload code, as well as how to allow the unsafe call.
This allow unsafe flag would need to be set to true by the developer at the start of the afterload method to allow the code to be executed without throwing an error.
After the afterload method has finished executing, the flag should be set back to it's state that it was in before the method was called (in a finally block).
This is to prevent another objects afterload resetting the flag halfway through an afterload method that is to allow unsafe code.

The MOST dangerous code would be calling a load for the same type of BO that the current afterload is executing for which potentially results in an infinite loop (there have been other solutions proposed for this, like a "AfterLoadCurrentlyExecuting" flag on each BO).
(-Mark)

Brett & Peter what do you guys think if this "AllowUnsafeAfterLoad" solution?

Updated by Peter Wiles about 3 years ago
This seems like a reasonable solution to me - I have seen some hectic code in AfterLoads (and written some crazy afterloads myself which always caused issues), so it would be good to at least indicate to devs that this is not a good idea.
How does the loading code know that it's being called from an AfterLoad? (unless the loading code sets a setting before calling the AfterLoad, I guess this could work).
Where do you think we could put this AllowUnsafeAfterLoad setting? In Faces I created a Settings dictionary in the UISettings class - maybe some sort of settings object in the GlobalRegistry?
Peter

Updated by Peter Wiles about 1 hour ago
Think this can be effected with a really good comprehensive comment on the AfterLoad method explaining that it should not be used for intensive operations (such as further loads etc). It's intended for setting up simple calculated fields and the like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant