Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 71bdf282 authored by Tom Cherry's avatar Tom Cherry
Browse files

init: Handle properties in the background of calling fs_mgr 

It's been a long standing problem that init calls fs_mgr functions
synchronously and therefore stops handling properties, which causes
deadlocks if either fs_mgr, or vdc, or vold attempt to set
properties.

Previous work, b/21904461, shows that there is a large performance
penalty for adding any amount of locking to properties, so moving
property service into its own thread generically is not a viable
option.  However, we can be sure that init is not setting properties
while the fs_mgr functions are running, so we can poll the property
socket in a thread while we call these functions.

The other alternative would have been to separate the fs_mgr functions
into smaller pieces and revisit the main init loop between each
piece.  Unfortunately, this would be difficult, since
fs_mgr_mount_all() calls out to different processes via logwrapper,
which synchronously polls on a logging FD from the child, among other
complexities that would make this strategy much more difficult than it
would be worth.

Bug: 21904461
Test: device boots, including when setting property in
      fs_mgr_mount_all()

Change-Id: Ib0b7123024035884f9d90f9b489c1e2f5a2e1707
parent 99087dec

init/builtins.cpp

+21 −6
Original line number Diff line number Diff line
@@ -588,7 +588,12 @@ static Result<void> do_mount_all(const BuiltinArguments& args) { 
    if (!ReadFstabFromFile(fstab_file, &fstab)) {

        return Error() << "Could not read fstab";

    }

    auto mount_fstab_return_code = fs_mgr_mount_all(&fstab, mount_mode);



    auto mount_fstab_return_code =

            CallFunctionAndHandleProperties(fs_mgr_mount_all, &fstab, mount_mode);

    if (!mount_fstab_return_code) {

        return Error() << "Could not call fs_mgr_mount_all(): " << mount_fstab_return_code.error();

    }

    property_set(prop_name, std::to_string(t.duration().count()));



    if (import_rc && SelinuxGetVendorAndroidVersion() <= __ANDROID_API_Q__) {
@@ -599,7 +604,7 @@ static Result<void> do_mount_all(const BuiltinArguments& args) { 
    if (queue_event) {

        /* queue_fs_event will queue event based on mount_fstab return code

         * and return processed return code*/

        auto queue_fs_result = queue_fs_event(mount_fstab_return_code);

        auto queue_fs_result = queue_fs_event(*mount_fstab_return_code);

        if (!queue_fs_result) {

            return Error() << "queue_fs_event() failed: " << queue_fs_result.error();

        }
@@ -615,8 +620,13 @@ static Result<void> do_umount_all(const BuiltinArguments& args) { 
        return Error() << "Could not read fstab";

    }



    if (auto result = fs_mgr_umount_all(&fstab); result != 0) {

        return Error() << "umount_fstab() failed " << result;

    auto result = CallFunctionAndHandleProperties(fs_mgr_umount_all, &fstab);

    if (!result) {

        return Error() << "Could not call fs_mgr_mount_all() " << result.error();

    }



    if (*result != 0) {

        return Error() << "fs_mgr_mount_all() failed: " << *result;

    }

    return {};

}
@@ -627,8 +637,13 @@ static Result<void> do_swapon_all(const BuiltinArguments& args) { 
        return Error() << "Could not read fstab '" << args[1] << "'";

    }



    if (!fs_mgr_swapon_all(fstab)) {

        return Error() << "fs_mgr_swapon_all() failed";

    auto result = CallFunctionAndHandleProperties(fs_mgr_swapon_all, fstab);

    if (!result) {

        return Error() << "Could not call fs_mgr_swapon_all() " << result.error();

    }



    if (*result == 0) {

        return Error() << "fs_mgr_swapon_all() failed.";

    }



    return {};

init/property_service.cpp

+40 −1
Original line number Diff line number Diff line
@@ -39,6 +39,7 @@ 
#define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_

#include <sys/_system_properties.h>



#include <atomic>

#include <map>

#include <memory>

#include <mutex>
@@ -52,6 +53,7 @@ 
#include <android-base/properties.h>

#include <android-base/stringprintf.h>

#include <android-base/strings.h>

#include <android-base/unique_fd.h>

#include <property_info_parser/property_info_parser.h>

#include <property_info_serializer/property_info_serializer.h>

#include <selinux/android.h>
@@ -59,7 +61,6 @@ 
#include <selinux/selinux.h>



#include "debug_ramdisk.h"

#include "epoll.h"

#include "init.h"

#include "persistent_properties.h"

#include "property_type.h"
@@ -76,6 +77,7 @@ using android::base::StartsWith; 
using android::base::StringPrintf;

using android::base::Timer;

using android::base::Trim;

using android::base::unique_fd;

using android::base::WriteStringToFile;

using android::properties::BuildTrie;

using android::properties::ParsePropertyInfoFile;
@@ -1006,5 +1008,42 @@ void StartPropertyService(Epoll* epoll) { 
    }

}



Result<int> CallFunctionAndHandlePropertiesImpl(const std::function<int()>& f) {

    unique_fd reader;

    unique_fd writer;

    if (!Socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, &reader, &writer)) {

        return ErrnoError() << "Could not create socket pair";

    }



    int result = 0;

    std::atomic<bool> end = false;

    auto thread = std::thread{[&f, &result, &end, &writer] {

        result = f();

        end = true;

        send(writer, "1", 1, 0);

    }};



    Epoll epoll;

    if (auto result = epoll.Open(); !result) {

        return Error() << "Could not create epoll: " << result.error();

    }

    if (auto result = epoll.RegisterHandler(property_set_fd, handle_property_set_fd); !result) {

        return Error() << "Could not register epoll handler for property fd: " << result.error();

    }



    // No-op function, just used to break from loop.

    if (auto result = epoll.RegisterHandler(reader, [] {}); !result) {

        return Error() << "Could not register epoll handler for ending thread:" << result.error();

    }



    while (!end) {

        epoll.Wait({});

    }



    thread.join();



    return result;

}



}  // namespace init

}  // namespace android

init/property_service.h

+10 −0
Original line number Diff line number Diff line
@@ -18,9 +18,11 @@ 


#include <sys/socket.h>



#include <functional>

#include <string>



#include "epoll.h"

#include "result.h"



namespace android {

namespace init {
@@ -37,5 +39,13 @@ void property_load_boot_defaults(bool load_debug_prop); 
void load_persist_props();

void StartPropertyService(Epoll* epoll);



template <typename F, typename... Args>

Result<int> CallFunctionAndHandleProperties(F&& f, Args&&... args) {

    Result<int> CallFunctionAndHandlePropertiesImpl(const std::function<int()>& f);



    auto func = [&] { return f(args...); };

    return CallFunctionAndHandlePropertiesImpl(func);

}



}  // namespace init

}  // namespace android