----- Original Message -----
> TS-2302: convert Log::error_log into a generic prefedined LogObject
>
> Initialize LogObject pointer arrays to NULL before using them. Use
> const accessors to get to LogObject::m_name. Sprinkle more const
> through the logging API.
>
> Use the standard ConfigProcessor pattern to reload LogConfig. This
> removes part of the need to keep a global array of inactive LogObjects.
> Using newDeleter() in LogObjectManager::unmanage_api_object(),
> removes the rest of this need. Now the whole Log::inactive_object
> array can be removed.
>
> Using the ConfigProcessor reload pattern also fixes a memory leak,
> since previously the old LogConfig object was never freed.
>
> Fix LogObjectManager::transfer_objects(). Previously, matching
> objects would be assigned to the new manager, but also left on the
> old manager, leaving it unclear who should have ownership of which
> log object.
>
> Move a bunch of inline LogObjectManager methods to the .cc file
> where they are mre easily accessible. None of these are called often
> enough to justify being inline.
>
> The global Log::error_log is now managed my the log object manager
> just like other predefined log objects. This lets us remove code
> in a number of places that were treating is specially. It also fixes
> a log collation bug in LogConfig::init(). This function attempts
> to figure out which log space threshold to use. If all the logs are
> being collated, then it used the orphan space value. The problem
> is that the error log was never checked (and it is never collated),
> so enabling log collation and making a poor choice of space thresholds
> causes error logging to stop. Like the rest of the bugs here, the
> fix is to stop treating the error log specially, and bring it into
> the log object manager.
>
>
[snip]
> Commit: 538eba5c078f6b14cbd3b6e6887da669fd7205c9
[snip]
> diff --git a/proxy/logging/Log.h b/proxy/logging/Log.h
> index 883567f..a646275 100644
> --- a/proxy/logging/Log.h
> +++ b/proxy/logging/Log.h
> @@ -404,7 +404,7 @@ public:
> inkcoreapi static int error(const char *format, ...) TS_PRINTFLIKE(1, 2);
>
> // public data members
> - inkcoreapi static TextLogObject *error_log;
> + inkcoreapi static LogObject *error_log;
> static LogConfig *config;
> static LogFieldList global_field_list;
> // static LogBufferList global_buffer_full_list; vl: not used
> @@ -414,12 +414,6 @@ public:
> static LogObject *global_scrap_object;
> static LoggingMode logging_mode;
>
> - // keep track of inactive objects
> - static LogObject **inactive_objects;
> - static size_t numInactiveObjects;
> - static size_t maxInactiveObjects;
> - static void add_to_inactive(LogObject * obj);
> -
> // logging thread stuff
> static EventNotify *preproc_notify;
> static void *preproc_thread_main(void *args);
> @@ -442,6 +436,7 @@ public:
>
> Log(); // shut up stupid DEC C++ compiler
Really? Do we really care about DEC's C++ Compiler?
> + friend void RegressionTest_LogObjectManager_Transfer(RegressionTest *,
> int, int *);
> private:
>
> static void periodic_tasks(long time_now);
>
[snip]
> @@ -1335,3 +1418,67 @@ LogObjectManager::log(LogAccess * lad)
>
> return ret;
> }
> +
> +void
> +LogObjectManager::flush_all_objects()
> +{
> + for (unsigned i = 0; i < this->_numObjects; ++i) {
> + if (this->_objects[i]) {
> + this->_objects[i]->force_new_buffer();
> + }
> + }
> +
> + for (unsigned i = 0; i < this->_numAPIobjects; ++i) {
> + if (this->_APIobjects[i]) {
> + this->_APIobjects[i]->force_new_buffer();
> + }
> + }
> +}
> +
> +#if TS_HAS_TESTS
> +
> +static LogObject *
> +MakeTestLogObject(const char * name)
> +{
> + const char * tmpdir = getenv("TMPDIR");
> + LogFormat format("testfmt", NULL);
> +
> + if (!tmpdir) {
> + tmpdir = "/tmp";
> + }
> +
> + return NEW(new LogObject(&format, tmpdir, name,
> + LOG_FILE_ASCII /* file_format */, name /* header */,
> + 1 /* rolling_enabled */, 1 /* flush_threads */));
> +}
> +
> +REGRESSION_TEST(LogObjectManager_Transfer)(RegressionTest * t, int /* atype
> ATS_UNUSED */, int * pstatus)
> +{
> + TestBox box(t, pstatus);
> +
> + // There used to be a lot of confusion around whether LogObjects were
> owned by ome or more LogObjectManager
Who is `ome` ?
> + // objects, or handed off to static storage in the Log class. This test
> just verifies that this is no longer
> + // the case.
> + {
> + LogObjectManager mgr1;
> + LogObjectManager mgr2;
> +
> + mgr1.manage_object(MakeTestLogObject("object1"));
> + mgr1.manage_object(MakeTestLogObject("object2"));
> + mgr1.manage_object(MakeTestLogObject("object3"));
> + mgr1.manage_object(MakeTestLogObject("object4"));
> +
> + mgr2.transfer_objects(mgr1);
> +
> + rprintf(t, "mgr1 has %d objects, mgr2 has %d objects\n",
> + (int)mgr1.get_num_objects(), (int)mgr2.get_num_objects());
> +
> + rprintf(t, "running Log::periodoc_tasks()\n");
> + Log::periodic_tasks(ink_get_hrtime() / HRTIME_SECOND);
> + rprintf(t, "Log::periodoc_tasks() done\n");
> + }
> +
> + box = REGRESSION_TEST_PASSED;
> +}
> +
> +#endif
[snip]
++ i
Igor Galić
Tel: +43 (0) 664 886 22 883
Mail: i.ga...@brainsware.org
URL: http://brainsware.org/
GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE
