[ADDED] scope guard for taken compileTraversals

[siystar]

Pull Request Template

Description

Added scope guard for compileTraversals taken by CompileManager to return them to the available queue on exception

Fixes # (issue)

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Tested that shader compile errors encountered in compile thread no longer freeze the application

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[timoore]

I like gsl::finally for this sort of thing, instead of writing a new struct every time you need a new guard.

[robertosfield]

Thanks for the fix. I have reviewed the changes and understand their intent but it adds complexity and looses clarity of what is happening in the code so I would like to look for a cleaner/more readable way of doing it I will leave this PR up until I come up with an alternative.

[robertosfield]

We should probably have try/catch around the compile traversal.

I am planning a proper review of vsg::CompileManager/CompileTraversal this week so can take this PR and the one from @timoore as reference for what issues need to encompassed in any changes I make.

[robertosfield]

@siystar I am currently looking at this PR.

The if (contextSelection) branch also has a paired compileTraversal->contexts.swap(contexts); will also be sensitive to an exception being thrown. So I think this should probably be tackled at the same time.

I will ponder what the cleanest way to do this will be. A try catch is one route. Using something equivalent to gsl::finally is another, though it'll have to be something standard rather than require another external dependency.

I should have something settle by the end of this afternoon.

[robertosfield]

I have implement a try/catch in the CompileManger::compile(..) method:

https://github.com/vsg-dev/VulkanSceneGraph/tree/CompileCatchException

I have also added a CompileResult::message string so we can pass back a message when the result value. This is just a first step with refactors of CompielManager that I think is needed, but I think it should be sufficient for addressing the issue that is the topic of this PR so I'm thinking this PR is now moot.

[robertosfield]

I have merged the CompileCatchException branch with VSG master with PR #794. I will close this PR as I think it's no longer required. If there are issues remaining then we can look at it again.