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

Fix crash caused by deleting buffer we do not own #28

Merged

Conversation

JosiahWI
Copy link

This PR fixes #27 by not deleting the buffer in the render core after we get it from the pipeline.

The documentation in pipeline.h,

    /**    
     * Create a new object that will be managed by the pipeline    
     *    
     * @tparam T type of the object to be created    
     * @tparam Args types of constructor arguments    
     * @param args constructor arguments    
     * @return T* pointer to the newly created object    
     */    
    template<typename T, typename... Args>
    T *createOwned(Args&&... args) {
        return own(std::make_unique<T>(std::forward<Args>(args)...));
    }

and

    /**    
     * Create a new object that will be managed by the pipeline    
     *    
     * @tparam T type of the object to be created    
     * @tparam Args types of constructor arguments    
     * @param args constructor arguments    
     * @return T* pointer to the newly created object    
     */    
    template<typename T, typename... Args>
    T *createOwned(Args&&... args) {
        return own(std::make_unique<T>(std::forward<Args>(args)...));
    }

makes it clear that the pipeline will delete the buffer. We are not supposed to.

To do

Ready for Review.

How to test

Compile and see that ./test_loop.py no longer hangs.

@Rabbidon Rabbidon merged commit e615079 into EleutherAI:headless-renderer Jan 12, 2023
@JosiahWI JosiahWI deleted the fix/render-core-bad-delete branch January 12, 2023 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants