Default Project CR-8433

Second round of fixes for OPENAM-7201

Closed on 03 Nov 15

  •  
  •  
  •  
  •  
  • Author & Moderator
  • Reviewers
    • Reviewer completed
    • Reviewer completed

CR-8433 9

Keyboard shortcuts  
Summarize the review outcomes (optional)
 
#permalink

Details

Warning: no files are visible, they have all been filtered.
Participant Role Time Spent Comments Latest Comment
Author & Moderator 55m 2 static am_bool_t unlink_sem(char *sem_name, void (*log_c...
Reviewer - Complete 27m 5 perfect. this works fine. CR complete for me.
Reviewer - Complete 14m 2 Completing the latest version. http://sources.forgerock.o...
Total   1h 37m 9  
#permalink

Objectives

This patch does the following:
1) make sure we do not call pthread_atfork again on restart (parent process is not changing);
2) make sure agent module is not unloaded from a parent process during restart (apache unloads/loads all modules on restart);
3) make sure openssl libs (when available and loaded by the agent) stay put during restart.

#permalink

Issues Raised From Comments

Key Summary State Assignee
#permalink

General Comments

03 Nov 15

Charles Sparey says:

LGTM, based upon the implementation in mod_perl and the patch document you sent me last week. Marking as complete.

03 Nov 15

Nicholas James says:

Changes required to test_int, test_log and test_notification
Also OS X doesn't like the -z nodelete

03 Nov 15

Nicholas James says:

I'm having a problem with tests/test_init test_init_cleanup.
This is meant to call am_init then check that all the resources can be cleaned up.
For some reason I'm getting no resources to clean up... I'm investigating - perhaps am_init isn't being called right?

03 Nov 15

Nicholas James says:

I'm having problems with mac version of test_init_cleardown after patch 3.
shm_unlink returns EINVAL, which is treated as an error by am_remove_shm_and_locks
I don't know the reason, but will now check on linux. May need to #ifndef APPLE for the sem clear down....
It looks like mac sem_unlink actually returns EINVAL when we would expect it to return ENOENT - it successfully removes the sem if its present. So – what to do?

03 Nov 15

Mareks Malnačs says:


static am_bool_t unlink_sem(char *sem_name, void (*log_cb)(void *arg, char *name, int error), void *cb_arg) {
    errno = 0;
    if (sem_unlink(sem_name) == 0) {
        // warn: semaphore was present but successfully cleared
        log_cb(cb_arg, sem_name, 0);
    } else if (errno != ENOENT
#ifdef __APPLE__
            && errno != EINVAL
#endif
            ) {
        // failure: semaphore was not cleared
        log_cb(cb_arg, sem_name, errno);
        return AM_FALSE;
    }
    return AM_TRUE;
}

03 Nov 15

Nicholas James says:

perfect. this works fine.
CR complete for me.

03 Nov 15

Charles Sparey says:

Completing the latest version.

/source/apache/agent.c Changed  
Open in IDE #permalink
/source/varnish/agent.c Changed  
Open in IDE #permalink
/source/varnish3/agent.c Changed  
Open in IDE #permalink
/source/am.h Changed  
Open in IDE #permalink
/source/init.c Changed  
Open in IDE #permalink
/source/net_client_ssl.c Changed  
Open in IDE #permalink
/source/thread.c Changed  
Open in IDE #permalink
/source/thread.h Changed  
Open in IDE #permalink
/Makefile.linux.mk Changed  
Open in IDE #permalink
/Makefile.macos.mk Changed  
Open in IDE #permalink
/Makefile.solaris.mk Changed   2
Open in IDE #permalink
/source/apache/agent.c Changed  
Open in IDE #permalink
/source/varnish/agent.c Changed  
Open in IDE #permalink
/source/varnish3/agent.c Changed  
Open in IDE #permalink
/source/am.h Changed  
Open in IDE #permalink
/source/init.c Changed  
Open in IDE #permalink
/source/net_client_ssl.c Changed  
Open in IDE #permalink
/source/thread.c Changed  
Open in IDE #permalink
/source/thread.h Changed  
Open in IDE #permalink
/tests/test_init.c Changed  
Open in IDE #permalink
/tests/test_log.c Changed  
Open in IDE #permalink
/tests/test_notifications.c Changed  
Open in IDE #permalink
/Makefile.linux.mk Changed  
Open in IDE #permalink
/Makefile.solaris.mk Changed  
Open in IDE #permalink
/source/apache/agent.c Changed  
Open in IDE #permalink
/source/varnish/agent.c Changed  
Open in IDE #permalink
/source/varnish3/agent.c Changed  
Open in IDE #permalink
/source/am.h Changed  
Open in IDE #permalink
/source/init.c Changed  
Open in IDE #permalink
/source/net_client_ssl.c Changed  
Open in IDE #permalink
/source/thread.c Changed  
Open in IDE #permalink
/source/thread.h Changed  
Open in IDE #permalink
/tests/test_init.c Changed  
Open in IDE #permalink
/tests/test_log.c Changed  
Open in IDE #permalink
/tests/test_notifications.c Changed  
Open in IDE #permalink
/Makefile.linux.mk Changed  
Open in IDE #permalink
/Makefile.solaris.mk Changed  
Open in IDE #permalink

Review updated: Reload | Ignore | Collapse

You cannot reload the review while writing a comment.

Create Issue

X
Assign To Me

Log time against