Skip to content

Add delete to vulnreport endpoint #465

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

Merged
merged 1 commit into from
Mar 22, 2025

Conversation

git-hyagi
Copy link
Collaborator

@git-hyagi git-hyagi commented Mar 21, 2025

@bmbouter
Copy link
Member

There could be other errors in the background thread that cause it to die and could cause MainThread to deadlock... Consider this AI response and example about how to make this pattern more durable (several ways).

Handling exceptions in a background thread and ensuring the main thread does not deadlock when waiting on a queue can be tricky. Here's a structured approach to handle this gracefully:
Key Considerations:

    Exception Propagation: Ensure exceptions from the background thread are propagated to the main thread.

    Queue Timeout: Use a timeout when waiting on the queue to avoid indefinite blocking.

    Thread State Monitoring: Check if the background thread is still alive before waiting on the queue.

    Graceful Shutdown: Ensure the background thread can signal the main thread to stop waiting.

Implementation Example
python
Copy

import threading
import queue
import time
import sys

def background_task(q):
    try:
        for i in range(5):
            print(f"Background thread working: {i}")
            time.sleep(1)
            if i == 3:
                raise RuntimeError("Simulated error in background thread")
            q.put(i)
    except Exception as e:
        # Put the exception in the queue to notify the main thread
        q.put(e)
    finally:
        # Signal that the thread is done
        q.put(None)

def main():
    q = queue.Queue()
    thread = threading.Thread(target=background_task, args=(q,))
    thread.start()

    while True:
        try:
            # Wait for items with a timeout to avoid deadlock
            item = q.get(timeout=1)
            if item is None:
                print("Background thread finished.")
                break
            elif isinstance(item, Exception):
                print(f"Exception in background thread: {item}")
                break
            else:
                print(f"Main thread received: {item}")
        except queue.Empty:
            # Check if the background thread is still alive
            if not thread.is_alive():
                print("Background thread died unexpectedly.")
                break

    thread.join()  # Ensure the thread is cleaned up
    print("Main thread exiting.")

if __name__ == "__main__":
    main()

Explanation:

    Background Thread:

        The background thread performs its tasks and puts results into the queue.

        If an exception occurs, it puts the exception into the queue.

        It signals completion by putting None into the queue.

    Main Thread:

        Uses q.get(timeout=1) to wait for items with a timeout, avoiding indefinite blocking.

        Checks if the background thread is still alive if the queue is empty.

        Handles exceptions from the background thread by breaking the loop.

        Ensures the background thread is joined before exiting.

Key Points:

    Timeout: Using a timeout in q.get() ensures the main thread doesn't block indefinitely.

    Exception Handling: Exceptions from the background thread are propagated to the main thread via the queue.

    Graceful Shutdown: The background thread signals completion by putting None into the queue.

This approach ensures the main thread can handle exceptions and avoid deadlocks gracefully.

Copy link
Member

@bmbouter bmbouter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make the design more durable for the background thread experiencing any kind of exception.

@bmbouter
Copy link
Member

@git-hyagi if you want to move the bugfix to its own PR to get the delete in more quickly feel free. That part looks fine. Or not, it's up to you.

@git-hyagi
Copy link
Collaborator Author

@bmbouter thank you for all the explanations and help!!
I am going to move the bugfix to its own PR, since it is not that simple.

@git-hyagi git-hyagi force-pushed the add-destroy-to-vulnreport branch from 130f869 to e594b94 Compare March 21, 2025 17:02
@git-hyagi git-hyagi changed the title Add delete to vulnreport report Add delete to vulnreport endpoint Mar 21, 2025
Copy link
Member

@bmbouter bmbouter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@bmbouter bmbouter merged commit fbf424f into pulp:main Mar 22, 2025
1 check passed
@git-hyagi git-hyagi deleted the add-destroy-to-vulnreport branch March 24, 2025 10:40
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.

2 participants