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

Revert "Solve Kombu filesystem transport not thread safe" #1595

Merged
merged 1 commit into from
Sep 9, 2022

Conversation

karajan1001
Copy link
Contributor

@karajan1001 karajan1001 commented Sep 9, 2022

Reverts #1593
Sorry, the PR created by me works even worse than previous on our program. It only solved part of the problem and introduce a new one. I'll create a new PR later.

The previous problem comes from:

  1. in _queue_bind
    def _queue_bind(self, exchange, routing_key, pattern, queue):
        queues = self.get_table(exchange)
        queue_val = exchange_queue_t(routing_key or "", pattern or "",
                                     queue or "")
        if queue_val not in queues:
            queues.insert(0, queue_val)
        with self._get_exchange_file_obj(exchange, "wb") as f_obj:
            f_obj.write(str_to_bytes(dumps(queues)))

We first read the table and then add a new line to it. We do add a lock during both progress but released it too quickly and the file might change between these two progress.
2. in _get and _purge we modified the data but didn't add a lock to them.

  1. The new problem introduced by this PR.
    signal only works in the main thread.

Actually, I didn't expect it to be merged so quickly. Next time I'll add draft sign to any PR that hadn'd been carefully tested.

I'll create a new PR next Monday.

@lgtm-com
Copy link

lgtm-com bot commented Sep 9, 2022

This pull request introduces 1 alert when merging 0daad6c into 8699920 - view on LGTM.com

new alerts:

  • 1 for Unused import

@auvipy auvipy merged commit afcde0a into celery:master Sep 9, 2022
@auvipy
Copy link
Member

auvipy commented Sep 9, 2022

better luck next time

@auvipy auvipy added this to the 5.3 milestone Sep 9, 2022
@karajan1001 karajan1001 deleted the revert-1593-fix398 branch September 10, 2022 08:06
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