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 race condition in setting ReferenceCountedOpenSslEngine.needTask flag #12140

Closed

Conversation

kwart
Copy link

@kwart kwart commented Mar 2, 2022

Fixes #12139

What's changed

This PR reorders resetting ReferenceCountedOpenSslEngine.needTask flag within the io.netty.handler.ssl.ReferenceCountedOpenSslEngine.TaskDecorator.run() method. The flag shouldn't be unset before the task execution finishes.

What's not changed

This PR doesn't change the needTask flag handling in AsyncTasks. I'm not sure when and how this type is used.

There could be also a problem if more tasks are allowed to be returned for subsequent getDelegatedTask() call.

    List<Runnable> collectTasks(SSLEngine sslEngine) {
        List<Runnable> tasks = new LinkedList<Runnable>();
        Runnable task;
        while ((task = sslEngine.getDelegatedTask()) != null) {
            tasks.add(task);
        }
        return tasks;
    }

Then the single boolean variable is not enough to track task processing. It would rather deserve a task counter.

@chrisvest
Copy link
Contributor

It was specifically changes to this in #11739 because there are other issues if the task in question inspects the needTask field.
I don't recall the details of that, but just saying there's more going on and we shouldn't fix one bug by bringing back another.

@normanmaurer
Copy link
Member

Let me investigate...

@normanmaurer normanmaurer self-assigned this Mar 3, 2022
@normanmaurer
Copy link
Member

working on this now

@normanmaurer
Copy link
Member

closing... This is replaced by #12149

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.

Delegated tasks issue in ReferenceCountedOpenSslEngine
3 participants