Remove threading before process forking

Bug #1569404 reported by Dmitriy Ukhlov
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
neutron
High
Dmitriy Ukhlov

Bug Description

 Forking processes when a few threads are already running is potentially unsafe operation and could cause a lot of problems because only current thread will continue working in child thread. Any locked by other thread resource will remain locked forever.

We faced with this problem during oslo.messaging development and added workaround to hide this problem: https://review.openstack.org/#/c/274255/ I tried to fix this problem in oslo.service: https://review.openstack.org/#/c/270832/ but oslo folks said that this fix is ugly and it is wrong way to add workarounds to common libraries because projects use them incorrectly. I think that is fare.

Dmitriy Ukhlov (dukhlov)
Changed in neutron:
assignee: nobody → Dmitriy Ukhlov (dukhlov)
status: New → In Progress
Revision history for this message
Ihar Hrachyshka (ihar-hrachyshka) wrote :

Marked as High since it blocks oslo.messaging team to proceed on Pika driver.

tags: added: oslo
Changed in neutron:
importance: Undecided → High
milestone: none → newton-1
Revision history for this message
Ihar Hrachyshka (ihar-hrachyshka) wrote :
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

Reviewed: https://review.openstack.org/276842
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=1cafff087194711399ad2a85a9f394f7204d7bdd
Submitter: Jenkins
Branch: master

commit 1cafff087194711399ad2a85a9f394f7204d7bdd
Author: dukhlov <email address hidden>
Date: Fri Feb 5 19:25:42 2016 +0200

    Remove threading before process forking

    Forking a process when multiple threads are running is an unsafe
    operation and could cause a lot of problems because only current
    thread will continue working in child thread. Any locked by other
    thread resource will remain locked forever.

    We faced with this problem during oslo.messaging development and
    added workaround to hide this problem:
    https://review.openstack.org/#/c/274255/

    I tried to fix this problem in oslo.service:
    https://review.openstack.org/#/c/270832/

    but oslo folks said that this fix is ugly and it is wrong way to add
    workarounds to common libraries because projects use them incorrectly.
    I think that is fair.

    So this patch fixes incorrect usage of oslo libraries. In this patch
    I extended functionality of NeutronWorker and add there
    `worker_process_count` parameter which determines how many processes
    should be spawned for this worker. If `worker_process_count` = 0 - don't
    create process and spawn thread in scope of current process for worker

    Then I moved all background tasks to workers and return them by
    `get_workers` method. start_plugin_workers collects plugin's workers
    using `get_workers` method and starts in ProcessLauncher first workers
    with `worker_process_count` > 0 and only after this starts threaded
    workers by simple Launcher

    Closes-bug: #1569404

    Change-Id: I0544f1d47ae53d572adda872847a56fa0b202d2e

Changed in neutron:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/313088

Revision history for this message
Armando Migliaccio (armando-migliaccio) wrote :

This is being reverted.

Changed in neutron:
status: Fix Released → Confirmed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on neutron (master)

Change abandoned by Richard Theis (<email address hidden>) on branch: master
Review: https://review.openstack.org/313088

Changed in neutron:
status: Confirmed → In Progress
Revision history for this message
Doug Hellmann (doug-hellmann) wrote : Fix included in openstack/neutron 9.0.0.0b1

This issue was fixed in the openstack/neutron 9.0.0.0b1 development milestone.

Changed in neutron:
milestone: newton-1 → newton-2
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

Reviewed: https://review.openstack.org/313277
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=483c5982c020ff21ceecf1d575c2d8fad2937d6e
Submitter: Jenkins
Branch: master

commit 483c5982c020ff21ceecf1d575c2d8fad2937d6e
Author: Dmitriy Ukhlov <email address hidden>
Date: Fri May 6 08:41:07 2016 +0000

    Revert "Revert "Remove threading before process forking""

    This reverts commit b1cdba1696f5d4ec71d37a773501bd4f9e0cddb9

    Original patch was reverted because it broke neutron plugin's
    backward compatibility and needed more work.

    This patch fixes that problems:
    1) original behaviour of add_agent_status_check,
       start_periodic_l3_agent_status_check and
       start_periodic_dhcp_agent_status_check methods is deprecated but kept
       for using in third part plugins for backward compatibility
    2) new add_agent_status_check_worker, add_periodic_l3_agent_status_check
       and add_periodic_dhcp_agent_status_check method are implemented
       instead and are used for implementing plugins in neutron codebase

    Closes-Bug: #1569404

    Change-Id: I3a32a95489831f0d862930384309eefdc881d8f6

Changed in neutron:
status: In Progress → Fix Released
tags: added: neutron-proactive-backport-potential
Revision history for this message
Doug Hellmann (doug-hellmann) wrote : Fix included in openstack/neutron 9.0.0.0b2

This issue was fixed in the openstack/neutron 9.0.0.0b2 development milestone.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/379780

tags: removed: neutron-proactive-backport-potential
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron (master)

Reviewed: https://review.openstack.org/379780
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=05cd620389aad58d9d1bdb7064dc0ec3034a4e46
Submitter: Jenkins
Branch: master

commit 05cd620389aad58d9d1bdb7064dc0ec3034a4e46
Author: Dariusz Smigiel <email address hidden>
Date: Thu Sep 29 19:22:24 2016 +0000

    Removed deprecated methods for AgentSchedulers

    Removed methods deprecated by
    change I3a32a95489831f0d862930384309eefdc881d8f6

    Change-Id: I00c8c323135e3510896a9b66877fe887285d9281
    Related-Bug: #1569404

tags: added: neutron-proactive-backport-potential
tags: removed: neutron-proactive-backport-potential
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers