Bug 44386 - NTEventLogAppender.dll for windows 64
Summary: NTEventLogAppender.dll for windows 64
Status: REOPENED
Alias: None
Product: Log4j - Now in Jira
Classification: Unclassified
Component: Appender (show other bugs)
Version: 1.2
Hardware: Other Windows Server 2003
: P2 normal
Target Milestone: ---
Assignee: log4j-dev
URL:
Keywords:
Depends on:
Blocks: 43313
  Show dependency tree
 
Reported: 2008-02-10 04:56 UTC by y360
Modified: 2012-04-06 18:26 UTC (History)
2 users (show)



Attachments
AMD64 and Itanium support for NTEventLogAppender (35.57 KB, application/octet-stream)
2008-06-11 15:25 UTC, David Bennion
Details
Better AMD64 and Itanium support for NTEventLogAppender patch (119.13 KB, application/x-zip-compressed)
2008-06-17 11:15 UTC, David Bennion
Details

Note You need to log in before you can comment on or make changes to this bug.
Description y360 2008-02-10 04:56:47 UTC
A version of NTEventLogAppender.dll should be built for Windows 64 bit OS as well

Some discussion took place a while ago
http://www.mail-archive.com/log4j-dev@logging.apache.org/msg08079.html
Comment 1 David Bennion 2008-06-11 15:25:35 UTC
Created attachment 22111 [details]
AMD64 and Itanium support for NTEventLogAppender

Here is the patch.  A few changes to the C code and a few changes to the java code.

Everything seems to be working for me.
Comment 2 David Bennion 2008-06-11 15:37:30 UTC
The attached patch is for the consideration of log4j development to merge into the main tree.  


I have included a file in the attachment called patch_readme.txt that details the changes.

Users who wanted to use this patch today could follow the following steps:

1. obtain log4j-1.2.15.jar from the regular distribution
2. compile the NTEventLogAppender.java against it using a simple javac
javac -cp log4j-1.2.15.jar NTEventLogAppender.java
3. take that NTEventLogAppender.class and put it in a jar file (keeping the org.apache.log4j.nt path/directory structure up front).
4. Make sure that the jar file you created in 3 is in the class path before log4j-1.2.15.jar so that it can override the NTEventLogAppender with the one from the patch.
5. Add the .dll's in the patch to the current working directory of your application (wherever you app is started from).


Comment 3 David Bennion 2008-06-17 11:15:54 UTC
Created attachment 22133 [details]
Better AMD64 and Itanium support for NTEventLogAppender patch

This fixes a problem with the dll's being built by VS2005 to require a dll included in the .NET Framework 2.0
Comment 4 David Bennion 2008-06-17 11:21:57 UTC
I made updates to the readme inside the patch, but the long and short of this second patch is this:

Visual Studio 2005 was forcing a library reference to a .dll in .NET Framework 2.0 which is really silly.  So I built the dlls statically.  They now work on systems without the .NET 2.0 framework.

I also created an AppenderNT.jar so that users could take the contents of this patch and make their log4j work on 64 bit windows with nt event logging today, while we wait for this to be made part of a main release of log4j.

Another minor change:  there is no reason for the .dlls to be com dll's that need to be regsvr32'ed on the systems.  So that has been removed.

Should I assign this to someone specific to get them to review and add the changes to source control??

Comment 5 David Bennion 2008-06-17 14:32:33 UTC
I made updates to the readme inside the patch, but the long and short of this second patch is this:

Visual Studio 2005 was forcing a library reference to a .dll in .NET Framework 2.0 which is really silly.  So I built the dlls statically.  They now work on systems without the .NET 2.0 framework.

I also created an AppenderNT.jar so that users could take the contents of this patch and make their log4j work on 64 bit windows with nt event logging today, while we wait for this to be made part of a main release of log4j.

Another minor change:  there is no reason for the .dlls to be com dll's that need to be regsvr32'ed on the systems.  So that has been removed.

Should I assign this to someone specific to get them to review and add the changes to source control??

Comment 6 Thorbjørn Ravn Andersen 2008-08-02 14:16:19 UTC
As it is very hard to build the DLL's on other platforms I suggest that they should be placed in subversion so the distribution can be built on any platform.
Comment 7 Curt Arnold 2008-08-21 16:03:07 UTC
Committed a different take in bug 687885.  The submitted patches changed the signatures on the native methods which could potentially result in undesirable behavior if the new Java class was mixed with an old DLL or vice-versa.

In the committed change, there are separate set of native methods (registerEventSource64 et al) that take long for the message source that are used they appear in the DLL.  If there is a UnsatisifedLinkError on ...64 method, the old "int" methods are called which from my testing allowed the new NTEventLogAppender.class work with an old DLL.

From comment #4:
>Another minor change:  there is no reason for the .dlls to be com dll's that
>need to be regsvr32'ed on the systems.  So that has been removed.

The registry keys that point to the category files are attempted to be set at run time if they are not present, however that user may not have privileges to write to the registry.  The DllRegisterServer allows the registry keys to be written at installation time.

From comment #6:
>As it is very hard to build the DLL's on other platforms I suggest that they
>should be placed in subversion so the distribution can be built on any
>platform.

Building NTEventLogAppender.dll for x86 on Linux and similar should be as simple as installing mingw-dev prior to running Maven.  However, it does look like MinGW's support for x86_64 is experimental at best (haven't looked at Itanium), so it does not seem practical attempt to build those DLL's with MinGW either on Windows or Linux.

To build NTEventLogAppender.dll for x86_64, open the appropriate command prompt window from the Visual Studio Tools entry in the start menu and then run:

mvn -Dntdll_target=msbuild package

The NTEventLogAppender unit tests will require running under an x86_64 JVM to succeed.  Though the DLL would be correctly build from either an x86 or x86_64 JVM.

It would be helpful to include an x86_64 DLL in the release, however it is not practical since the MinGW cross tools seem too immature and moving back to building on Windows is undesirable.  In addition the general ASF position is to not to provide platform specific binaries and let downstream packagers do that.

The class initializer first attempts to load NTEventLogAppender.dll.  If that fails (either missing or not the right type), it will then attempt to load NTEventLogAppender + System.getProperty("os.arch") + ".dll" which results in attempting to load NTEventLogAppenderamd64.dll when running with Sun's x86_64 JVM.  I thought the lower casing was problematic (turkish i and similar problems) and unnecessary since Windows file systems are case-insensitive.  Overriding the os.arch with a system property seemed overkill and potentially a security issue since you could redirect to an unexpected DLL. 

I've modified nteventlog.cpp so that it will still properly register itself if you rename the DLL.  So if you want to rename the DLL from NTEventLogAppender.dll to NTEventLogAppenderamd64.dll it will still work.

I thought about also providing a Visual Studio project file, however it would still require tweaking to include the right JVM include files and a custom step to invoke javah, etc.

Would appreciate feedback.
Comment 8 David Bennion 2008-08-21 18:34:04 UTC
>Committed a different take in bug 687885.  The submitted patches changed the
>signatures on the native methods which could potentially result in undesirable
>behavior if the new Java class was mixed with an old DLL or vice-versa.

You can do what you want, but I just don't believe this is an issue at all and I would take the approach I took.  I think it is unnecessary to create alternate 64 bit versions of the functions.  My rationale is this: If you are using the Windows NT Event Logging, you know that you need to deploy the .dlls when you put out a new version of log4j.  If you are not, it will never matter anyway as your app will never attempt to load the dll.

I think now is the time to make a clean break and do it right.  Keep the code simple and clean so that if someone was using this code as a model for some similar thing they were doing it would be obvious how to do it.

>>Another minor change:  there is no reason for the .dlls to be com dll's that
>>need to be regsvr32'ed on the systems.  So that has been removed.

>The registry keys that point to the category files are attempted to be set at
>run time if they are not present, however that user may not have privileges to
>write to the registry.  The DllRegisterServer allows the registry keys to be
>written at installation time.

I didn't think of that security privileges issue, and it is a valid point.  However, the idea of doing it via the DllRegisterServer doesn't guarantee a fix.  It only guarantees that you can write a default event source, so maybe named "log4j" or whatever you named it in your commit.  But that forces all applications to write to the "log4j" event source.  The user must specify that named event source, instead of choosing one that looks like his application.  If my application is ABCServer I would rather have my event logs show up as ABCServer rather than the generic log4j -- along with every other app that uses log4j event log appender.

The way I have it, all the user must do otherwise to initialize the event log is run the app as a privileged user one time so that it can create the event registry.  He could even run a simpleton "initialization" class instead of the whole app.  

I don't know what the golden solution to this is.  Personally, I think the DllRegisterServer is a misuse of the COM registration stuff, which is what regsvr32 is all about.  But it is a clever way to get stuff setup outside of any Java code.

>It would be helpful to include an x86_64 DLL in the release, however it is not
>practical since the MinGW cross tools seem too immature and moving back to
>building on Windows is undesirable.  In addition the general ASF position is to
>not to provide platform specific binaries and let downstream packagers do that.

Give me a break.  This is going to murder the value of this whole exercise for the average user.  The average log4j user is a java programmer -- not guaranteed at all to have the developer tools or know how to build a native stuff on Windows (especially since you need Visual Studio 2005 to build the 64 bit stuff and the itanium stuff and you don't have a reasonable mingw alternative now and probably for some time to come in the future).  Further, the user may want to support 64 bit windows but not have access 64 bit machines anyway to build the binary on in the first place.  Further yet, his working java logging application will cease to work when someone starts using a 64 bit JVM on 64 bit windows instead of 32 bit.  And the log4j programmer's user may need 64 bit due to the extra memory capacity available (i.e. above 1280M JVM memory), so then he is straddled with the choice between two features that he wants.

Just include the dlls.  The old package had the 32 bit dll pre-built and no one was complaining.  Just include the 64 bit dlls along with the source code and have done with the whole mess and get back to java development.  This has always been a fringe module for log4j anyway since it is not pure java.
Comment 9 Curt Arnold 2008-08-21 22:32:22 UTC
Changing the argument list on the existing native methods is not an option.  There are two many scenarios where you could have a mismatch between DLLs and log4j versions and bad things happen.  

However, it is not like we can't represent all the currently active event sources in 32-bits.  What we could do is return the HANDLE in the 32-bit implementation and maintain a small (maybe 256 entry) array of HANDLEs in the 64-bit versions and return the index into the array instead of the direct handle.  That would allow pre-1.2.16's log4j's to work on 64-bit JDK's as long as the NTEventLogAppender.dll on the path was a 64-bit DLL.  If you installed the 32-bit NTEventLogAppender.dll in \Windows\SysWOW64 and the 64-bit in \Windows\System32 (where 64 bit DLL's live despite the name), things might just magically work for older versions of log4j.  log4j 1.2.16 would be needed if you wanted to have a 32-bit dll and a 64-bit dll (NTEventLogAppender+ os.arch + .dll) both on the path.  I'll give that a shot tomorrow.

As for including the AMD64 and/or IA64 binaries.  If we were to include them in the log4j release itself, we'd have to keep canned versions of them in the SVN.  We are kind of doing the same thing now with the output of the message compiler since there isn't a cross-platform implementation of it.  It is a little distasteful compared to building everything from source in a release, but maybe tolerable.  I can build and test an AMD64 DLL and should be able to build an IA64, but I won't be able to test it.  If someone is willing to test the IA64 build, I'd be willing to include it in the release, but I would not feel good about including it untested.


Comment 10 y360 2008-08-22 01:17:19 UTC
> Give me a break.  This is going to murder the value of this whole exercise for
> the average user.  The average log4j user is a java programmer -- not
> guaranteed at all to have the developer tools or know how to build a native
> stuff on Windows

> Just include the dlls.

Curt, as the one who opened the bug, I completely agree with David Bennion's sentiment on this. Please try to find a way to include the dlls.
Comment 11 David Bennion 2008-08-22 06:54:26 UTC
>However, it is not like we can't represent all the currently active event >sources in 32-bits.  What we could do is return the HANDLE in the 32-bit >implementation and maintain a small (maybe 256 entry) array of HANDLEs in the >64-bit versions and return the index into the array instead of the direct >handle.  That would allow pre-1.2.16's log4j's to work on 64-bit JDK's as long >as the NTEventLogAppender.dll on the path was a 64-bit DLL.  If you installed >the 32-bit NTEventLogAppender.dll in \Windows\SysWOW64 and the 64-bit in
>\Windows\System32 (where 64 bit DLL's live despite the name), things might >just magically work for older versions of log4j.  log4j 1.2.16 would be needed >if you wanted to have a 32-bit dll and a 64-bit dll (NTEventLogAppender+ >os.arch +
>.dll) both on the path.  I'll give that a shot tomorrow.

I could see maintaining an array of HANDLEs.  Then the new dll returns an offset into the array instead of a pointer and the java side is none the wiser.  That would work and you'd likely never have a problem.  

Now I can see why you are so concerned about the signature of the Dll's!  It is because you are putting the DLL's in Windows\System32.  That's not something that I ever do.  I just leave them in the application path (with jars) and set java.library.path to load them.  

I would keep the different names for different arches though and based on the arch try to load the appropriate one.  That way people like me can simply stick all the .dll's in the same directory as their application.  Then their single application installation can switched JVM's in place.  For instance, the user can come and choose a different JVM for my application and could go to a 64 bit from right where it is installed.  

My feeling is that my java based installation should be able to just chuck all the .dll's on there and it should just "work" regardless of which JVM they are using.  I as a java application developer shouldn't have to put any energy into tracking whether I am set up for the 64 bit one or not.  Just slap them all on at once and go.

I think this model describes what at least a high percentage of other users will do as well.

It is generally considered distasteful to make dll additions to the system directory.  Microsoft wanted everyone to get away from that after so many applications experienced "dll hell" (which is what your signature changing concern is based on).  I realize that not everyone plays by those rules.

>I can build and test an AMD64 DLL and should be able to build an IA64, but I >won't be able to test it.  If someone is willing to test the IA64 build, I'd >be willing to include it in the release, but I would not feel good about >including it untested.

I agree.  I know there was a customer of ours that asked about Windows on Itanium, but by and large I think this is basically a dead platform.  I don't feel terribly bad about just leaving the code in a state where it builds on this platform.  I think there are a few people out there who have Windows on Itanium.  Maybe you could keep the IA64 .dll in an "already built" state and put a notation in the doc that if you are a developer and you wanted to support that platform you can download the IA64 .dll from such and such a link and try it to see if it works.

All the best.

David.
Comment 12 taohuang.tamu 2009-07-10 14:43:24 UTC
I am moving our web serviceon Tomcat to 64 bit. So I found this bug and used the patch in attachment. I followed the instructions in your patch_readme,
although the webservice and Tomcat 64bit both work fine, I don't see any logs output. Tomcat logs don't give any errors or complaints though. So would you please have a look of what I'm doing wrong? This is what I did:

1) build.xml I put AppenderNT.jar before log4j-1.2.14.jar
     ...
     <pathelement location="${AppenderNT-jar}"/>
     <pathelement location="${log4j-jar}"/>

2) put the NTEventLogAppender64.dll under tomcat\bin

3) put AppenderNT.jar under tomcat\lib

4) check tomcat\webapps\mine\WEB-INF\lib, verify that file
AppenderNT.jar is there

BTW, it's been almost a year since the last update on this bug. Any update? Will there be any official support of 64bit NTEventLogAppender from log4j?

Thanks!
Comment 13 Curt Arnold 2009-07-11 11:51:26 UTC
In newer OS's, you may need to explicitly register the DLL as admin, but I think that failing to do that would only result in the messages being formatted.

I think the DLL needs to be in the path of the process that launced java on on java.library.path (http://forums.sun.com/thread.jspa?threadID=780109).  I don't think placing the DLL on the java classpath is sufficient.

I guess my Hurricane Ike excuse is getting old.  Ike, marriage and a new job has been keeping me busy.  Won't promise but hopefully a nice surprise soon..
Comment 14 dB. 2010-09-28 14:18:58 UTC
Maybe consider scrapping NTEventLogAppender altogether and replacing it with a JNA-based implementation (see http://log4jna.codeplex.com).
Comment 15 spyhunter99 2012-04-06 18:26:39 UTC
On Win2k8 x64 with x64 JDK and the included NTAppender file in the Windows folder, I get an error stating that it can't load the IA 64 bit library on an AMD 64bit platform. Shrug. Works fine using the 32bit jdk

How has this not been fixed yet?