Implement auto attach

[TheAspens]

This pull request is a continuation of PR #2207 which has been merged.

[TheAspens]

From pull request #2207 after the merge, @CharlieFenton commented:

After discussion with @davidpanderson, I have modified the Async RPC code to increase the delay before displaying the "Communicating with client" dialog from 1.5 seconds to 60 second while the client is auto-attaching to a project. This allows more than ample time for the auto-attach before GUI RPCs are enabled. But we do display it after 60 seconds as a safety feature, so that the user can exit BOINC if the client hangs.

[CharlieFenton]

@davidpanderson: The last lines in GET_PROJECT_LIST_OP::handle_reply() are:

    // were we initiated by autologin?
    //
    gstate.process_autologin(false);

So this RPC will alway call process_autologin(false), even if the RPC was not initiated by process_autologin(true). This could cause different problems every time get_project_list_op.do_rpc() is called (typically every 14 days), depending on whether or not the static variables project_id and user_id were set when the client first started.

If project_id has never been set so it is not a valid project id, the Event log will display "Unknown project ID: project_id".

If project_id has never been set but it is a random valid project id, the client will attach to an unintended project. (Static variables are automatically initialized to 0 on the Mac; i don't know if this is true on all platforms.)

If there was an auto-attach when the client was first started, it will display "Already attached to project_name" .

I believe you need to add:
if (!autologin_in_progress) return; between lines 299 and 300 like so:

    } else {
        // here the get project list RPC finished.
        //
	if (!autologin_in_progress) return;
        // check whether it failed.
        //
        autologin_in_progress = false;

I suspect this is what you intended by the comment // were we initiated by autologin?

[AenBleidd]

@CharlieFenton,

If project_id has never been set but it is a random valid project id, the client will attach to an unintended project. (Static variables are automatically initialized to 0 on the Mac; i don't know if this is true on all platforms.)

Section 6.7.8 Initialization of C99 standard (n1256) answers this question:

If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate. If an object that has static storage duration is not initialized explicitly, then:

— if it has pointer type, it is initialized to a null pointer;

— if it has arithmetic type, it is initialized to (positive or unsigned) zero;

— if it is an aggregate, every member is initialized (recursively) according to these rules;

— if it is a union, the first named member is initialized (recursively) according to these rules.

[CharlieFenton]

@davidpanderson: i think a race condition might be possible if there is an auto-attach with a virgin install or with a new version of BOINC on top of an older version.

Say that the local all_projects_list.xml file does contain the project id in the token, so gstate.process_autologin(true) does not call get_project_list_op.do_rpc() but it does call lookup_login_token_op.do_rpc(). Since the lookup_login_token_op.do_rpc()is asynchronous, CLIENT_STATE::INIT() continues and calls all_projects_list_check(). Because this is a new version of the client, this calls get_project_list_op.do_rpc(). If the get_project_list_op RPC finishes before the lookup_login_token_op RPC, then GET_PROJECT_LIST_OP::handle_reply() will call gstate.process_autologin(false). I leave it to you to determine whether this unexpected re-entry of gstate.process_autologin() will cause any problems.

[davidpanderson]

Oops! Fixed.

[davidpanderson]

Good eye! That is indeed a problem. I committed a fix.

[CharlieFenton]

@davidpanderson: I see that the merge conflict is due to your removal of this code from client_state.cpp:

    if (gstate.projects.size() == 0) {
        msg_printf(NULL, MSG_INFO,
            "This computer is not attached to any projects"
        );
        msg_printf(NULL, MSG_INFO,
            "Visit http://boinc.berkeley.edu for instructions"
        );
    }

But I think we still want to display these messages when there is no login_token.txt file and no BOINC Manager is running (which would run the Attach Wizard.) There is probably no harm in displaying the messages even if the BOINC Manager displays the Attach Wizard. So I suggest keeping the above code, but executing it only if gstate.autologin_in_progress is false. This should also eliminate the merge conflict.

[davidpanderson]

The first message is OK.
The 2nd should be removed or changed because the BOINC web site front page doesn't have instructions for adding projects.

[CharlieFenton]

I restored the first message to be displayed only if not doing an autologin, and resolved the merge conflict. I believe this PR is now ready to be merged into Master, assuming that all checks pass.

[TheAspens]

@davidpanderson - if this is ready for final review, can you remove WIP from the title of the pull request?

@AenBleidd - was the item that you raised fixed?

@CharlieFenton or @davidpanderson have either of you built the server and client from the latest version branch and done testing on this or would you like me to do so before merging?

[AenBleidd]

@TheAspens, actually that was just an answer on @CharlieFenton question.
No other objections from my side.

[CharlieFenton]

@CharlieFenton or @davidpanderson have either of you built the server and client from the latest version branch and done testing on this or would you like me to do so before merging?

I haven't. I am waiting for the server code to be implemented on SETI@home to do further testing, since @davidpanderson wrote in PR #2241 16 days ago:

Once we get the server part in a vetted project (e.g. SETI@home) we can test with the standard project list. This should happen in a week or so. We won't release it for testing before then.

[TheAspens]

Will Erik deploy it to Seti@Home from this branch or is the intention for him to deploy from master after it is merged?

[TheAspens]

I've also now tested this and I'm ready to merge. Everyone who has commented on this pull request is not objecting either. @davidpanderson - if you can confirm that you want this merged now, then I will merge.

[davidpanderson]

OK to merge. SETI@home is in the process of upgrading their beta test software to master. They'll pull this in there, and then deploy on their production server, at which point we'll be able to do and end-to-end test and do a client test/release.

[RichardHaselgrove]

@SETIguy
Eric reported that the original Beta upgrade had caused problems with their downstream code (specifically workunit generators), but that problem is now resolved: he would deploy the code on their production server "in the next few days" - which could very well mean during maintenance today, judging by the context. I'll leave #2238 (which led me here) open for review after deployment, as now.